New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: Make sure all messages are dispatched #8264

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@eugeneo
Contributor

eugeneo commented Aug 24, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

This change only touches V8 inspector integration.

Description of change

This fixes a race condition when messages are coming while V8 is still
dispatching the previous batch.

CC: @ofrobots

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Aug 30, 2016

Member

LGTM but s/Make/make/ in the commit log status line.

Member

bnoordhuis commented Aug 30, 2016

LGTM but s/Make/make/ in the commit log status line.

@ofrobots

This comment has been minimized.

Show comment
Hide comment
Eugene Ostroukhov
inspector: make sure all messages are dispatched
This fixes a race condition when messages are coming while V8 is still
dispatching the previous batch.
@eugeneo

This comment has been minimized.

Show comment
Hide comment
@eugeneo

eugeneo Aug 30, 2016

Contributor

I filed the commit log line.

Contributor

eugeneo commented Aug 30, 2016

I filed the commit log line.

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Aug 30, 2016

Member

CI is red, looks like a build bot failure, trying again: https://ci.nodejs.org/job/node-test-pull-request/3896/

Member

jasnell commented Aug 30, 2016

CI is red, looks like a build bot failure, trying again: https://ci.nodejs.org/job/node-test-pull-request/3896/

@ofrobots

This comment has been minimized.

Show comment
Hide comment
@ofrobots

This comment has been minimized.

Show comment
Hide comment
@ofrobots

ofrobots Aug 31, 2016

Contributor

Modulo the expected failure on ubuntu1204-clang341, the CI is green.

Contributor

ofrobots commented Aug 31, 2016

Modulo the expected failure on ubuntu1204-clang341, the CI is green.

@ofrobots

This comment has been minimized.

Show comment
Hide comment
@ofrobots

ofrobots Aug 31, 2016

Contributor

Landed as 27fd2d7.

Contributor

ofrobots commented Aug 31, 2016

Landed as 27fd2d7.

@ofrobots ofrobots closed this Aug 31, 2016

ofrobots added a commit that referenced this pull request Aug 31, 2016

inspector: make sure all messages are dispatched
This fixes a race condition when messages are coming while V8 is still
dispatching the previous batch.

PR-URL: #8264
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>

@Fishrock123 Fishrock123 referenced this pull request Sep 6, 2016

Closed

v6.6.0 pre-proposal #8428

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 8, 2016

inspector: make sure all messages are dispatched
This fixes a race condition when messages are coming while V8 is still
dispatching the previous batch.

PR-URL: nodejs#8264
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>

Fishrock123 added a commit that referenced this pull request Sep 9, 2016

inspector: make sure all messages are dispatched
This fixes a race condition when messages are coming while V8 is still
dispatching the previous batch.

PR-URL: #8264
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>

@eugeneo eugeneo deleted the eugeneo:refresh branch Sep 16, 2016

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