Skip to content
This repository was archived by the owner on Oct 15, 2020. It is now read-only.

Update TTD for locations where AsyncHooks passes hidden state. - #252

Closed
mrkmarron wants to merge 1 commit into
nodejs:xplatfrom
mrkmarron:xplat
Closed

mrkmarron wants to merge 1 commit into
nodejs:xplatfrom
mrkmarron:xplat

Conversation

@mrkmarron

Copy link
Copy Markdown
Contributor
Affected core subsystem(s)

TTD, AsyncHooks

Description of change

AsyncHooks communicates between JS/Native using uncontrolled pointer writes. Add calls to notify TTD of these events.

Comment thread src/async-wrap.cc Outdated
@@ -1,4 +1,4 @@
// Copyright Joyent, Inc. and other Node contributors.
// Copyright Joyent, Inc. and other Node contributors.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is a BOM getting added or removed here?

@mrkmarron

Copy link
Copy Markdown
Contributor Author

Hi Kyle, I cleaned up the BOM (looking) issue and updated the PR.

Comment thread src/async-wrap.cc
#if ENABLE_TTD_NODE
if (s_doTTRecord || s_doTTReplay) {
unsigned int refcount = 0;
JsAddRef(*farray, &refcount);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is AsyncWrap a singleton? If not, is this a memory leak since I never see a JsRelease anywhere?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like it is a singleton, nevermind.

@kfarnung kfarnung left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants