Skip to content

Conversation

@eugeneo
Copy link
Contributor

@eugeneo eugeneo commented Jun 10, 2016

Checklist
  • make -j4 test (UNIX) or vcbuild test nosign (Windows) passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

This change only touches inspector connection

Description of change

In some cases close callback was called twice while other codepaths ended up
not releasing the memory at all.

CC: @ofrobots

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 10, 2016
@mscdex mscdex added the inspector Issues and PRs related to the V8 inspector protocol label Jun 10, 2016
@ofrobots
Copy link
Contributor

@ofrobots
Copy link
Contributor

Copy link
Member

Choose a reason for hiding this comment

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

The ASSERT_NE is useless now (and should have been a CHECK_EQ in hindsight.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. Thanks!

In some cases close callback was called twice, while in some cases the
memory was still not released at all.
@bnoordhuis
Copy link
Member

LGTM

@ofrobots
Copy link
Contributor

ofrobots pushed a commit that referenced this pull request Jun 17, 2016
In some cases close callback was called twice, while in some cases the
memory was still not released at all.

PR-URL: #7268
Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
@ofrobots
Copy link
Contributor

Landed as 9dfc8b9.

@ofrobots ofrobots closed this Jun 17, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
In some cases close callback was called twice, while in some cases the
memory was still not released at all.

PR-URL: #7268
Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
@eugeneo eugeneo deleted the no_double_dispose branch August 2, 2016 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants