Skip to content

fix possible app crash when used with meteor 2.0#1233

Merged
rochdev merged 3 commits into
masterfrom
async-resource-default
Feb 3, 2021
Merged

fix possible app crash when used with meteor 2.0#1233
rochdev merged 3 commits into
masterfrom
async-resource-default

Conversation

@rochdev

@rochdev rochdev commented Feb 3, 2021

Copy link
Copy Markdown
Member

What does this PR do?

Fix possible app crash in latest when used with Meteor 2.0 by updating the tracer to default to the async_resource scope manager since the issue seems to be a bug in AsyncLocalStorage.

Motivation

Applications using Meteor 2.0 with the tracer are experiencing random C++ errors. While it's still unclear why this is happening, switching to the executionAsyncResource based scope manager seems to resolve the issue.

Fixes #1229

@rochdev rochdev added bug Something isn't working core labels Feb 3, 2021
@rochdev
rochdev requested a review from a team as a code owner February 3, 2021 18:05
Comment thread packages/dd-trace/src/platform/node/index.js Outdated

@bengl bengl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@rochdev rochdev added this to the 0.30.6 milestone Feb 3, 2021
@rochdev
rochdev merged commit 077f23c into master Feb 3, 2021
@rochdev
rochdev deleted the async-resource-default branch February 3, 2021 22:19
@rochdev rochdev changed the title fix possible app crash in latest when used with meteor 2.0 fix possible app crash when used with meteor 2.0 Feb 3, 2021
@schmod

schmod commented Apr 2, 2021

Copy link
Copy Markdown
Contributor

Does anybody know if this was a performance regression, or if there are plans to restore AsyncLocalStorage as the default scope-manager? I'm a little hesitant to upgrade past the release that introduced this change, as we've been hit in the past by performance regressions when changes have been made to the tracer's scope-manager.

@rochdev

rochdev commented Apr 6, 2021

Copy link
Copy Markdown
Member Author

@schmod The new scope manager basically does the same thing that AsyncLocalStorage itself does under the hood, and simply removes any additional logic that we don't need, some of which resulted in an incompatibility with fibers which Meteor uses. Because of that, it would be extremely unlikely to be slower than AsyncLocalStorage since it's basically a subset of the same code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dd-trace causes crash in v8 internals, SIGABRT

3 participants