Skip to content

debugger: fix debugger blocked main thread - #2622

Closed
yjhjstz wants to merge 4 commits into
nodejs:masterfrom
yjhjstz:master-fix-debugger
Closed

yjhjstz wants to merge 4 commits into
nodejs:masterfrom
yjhjstz:master-fix-debugger

Conversation

@yjhjstz

@yjhjstz yjhjstz commented Aug 31, 2015

Copy link
Copy Markdown

see #2110
Call uv_walk to close all handles, otherwise would meat error below.

Assertion failed: ((err) == (0)), function Stop, file ../src/debug-agent.cc, line 156.

@thefourtheye

Copy link
Copy Markdown
Contributor

Did you mean to close #2151?

@yjhjstz
yjhjstz force-pushed the master-fix-debugger branch from 69cbbc4 to f811e80 Compare August 31, 2015 07:36
@yjhjstz

yjhjstz commented Aug 31, 2015

Copy link
Copy Markdown
Author

yes @thefourtheye , update PR #2151 in this pr , and add close handles. @indutny

@Fishrock123

Copy link
Copy Markdown
Contributor

ping @indutny

@yeerkkiller1

Copy link
Copy Markdown

Ditto ping @indutny, the inability to take a process out of debugging mode is a blocking issue on the project I am working on.

Comment thread test/fixtures/debug-connect.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

minor nit: can you change the require() statements to use const

@yjhjstz
yjhjstz force-pushed the master-fix-debugger branch from f811e80 to fe502d5 Compare January 25, 2016 07:12
@yjhjstz

yjhjstz commented Jan 25, 2016

Copy link
Copy Markdown
Author

any other suggestion?

Comment thread src/debug-agent.cc

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You don't need to add a method for this, you can just use a lambda function (or even reinterpret_cast<uv_walk_cb>(uv_close), although that's a bit of a hack.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

a bit hack.

Comment thread test/debugger/test-debugger-stop.js Outdated

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.

Can you make this common.fail().

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure

@yjhjstz
yjhjstz force-pushed the master-fix-debugger branch from 47c346d to 0f34fed Compare February 17, 2016 01:45
@yjhjstz
yjhjstz force-pushed the master-fix-debugger branch from 0f34fed to e0c48b7 Compare February 17, 2016 05:48
@yjhjstz

yjhjstz commented Feb 17, 2016

Copy link
Copy Markdown
Author

rebase and reopen #5270

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants