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
win,tty: fix deadlock caused by inconsistent state #2882
Conversation
The variable uv__read_console_status is left as IN_PROGRESS when the operation is cancelled ahead of time by the main thread requesting a trap (race condition?). This confuses the next call to uv__cancel_read_console(...) causing a deadlock due to a semaphore aquisition that is never released by the reading thread. Setting the status variable back to COMPLETE or NOT_STARTED fixes the issue. Fixes: nodejs/node#32999
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This indeed fixes the deadlock
The variable uv__read_console_status is left as IN_PROGRESS when the operation is canceled ahead of time by the main thread requesting a trap (race condition?). This confuses the next call to uv__cancel_read_console(...) causing a deadlock due to a semaphore acquisition that is never released by the reading thread. Setting the status variable back to COMPLETE or NOT_STARTED fixes the issue. Ref: nodejs/node#32999 PR-URL: #2882 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
my pleasure, |
So forgive me for being a noob but I am new to tracking and fixing bug via GitHib - I see landers found and fixed it and raised a commit. So I guess someone will accept that and it will be merged into the main branch? When that happens does this ticket get closed somehow and marked as resolved/fixed? And finally, how does it get pushed into the next (I assume next) version of Node? How can I track it has made it's way into a certain build so I can download and test myself? Thanks guys! |
@powershellwhizz this will be included in the next release of libuv. After the release, there will be a PR in Node with the updated libuv. After it lands, the next Node versions will ship with the bug fixed. It is a serious bug, so we will probably get this backported to older Node versions too. |
The variable uv__read_console_status is left as IN_PROGRESS when the operation is canceled ahead of time by the main thread requesting a trap (race condition?). This confuses the next call to uv__cancel_read_console(...) causing a deadlock due to a semaphore acquisition that is never released by the reading thread. Setting the status variable back to COMPLETE or NOT_STARTED fixes the issue. Ref: nodejs/node#32999 PR-URL: libuv#2882 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> (cherry picked from commit aeab873)
The variable uv__read_console_status is left as IN_PROGRESS when the operation is canceled ahead of time by the main thread requesting a trap (race condition?). This confuses the next call to uv__cancel_read_console(...) causing a deadlock due to a semaphore acquisition that is never released by the reading thread. Setting the status variable back to COMPLETE or NOT_STARTED fixes the issue. Ref: nodejs/node#32999 PR-URL: libuv#2882 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com> (cherry picked from commit aeab873)
The variable uv__read_console_status is left as IN_PROGRESS when the operation is canceled ahead of time by the main thread requesting a trap (race condition?). This confuses the next call to uv__cancel_read_console(...) causing a deadlock due to a semaphore acquisition that is never released by the reading thread. Setting the status variable back to COMPLETE or NOT_STARTED fixes the issue. Ref: nodejs/node#32999 PR-URL: libuv#2882 Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Hi,
This deadlocks is affecting nodejs in the way described in this issue:
nodejs/node#32999
in short, the REPL module calls uv_tty_set_mode 2 times when processing a line of code.
since libuv handles user input in chunks of 1024 bytes, copying and pasting more than 1024 bytes with some line breaks will cause nodejs to call uv_tty_set_mode while having pending data.
uv_tty_set_mode calls uv_tty_read_stop which calls uv__cancel_read_console and due to a race condition bug (described in the commit message) that prevents a semaphore to be released, the program deadlocks here
thanks @bzoz for giving insights of how the REPL module and tty works
and thanks to @powershellwhizz for uncovering the issue