win,tty: fix deadlock caused by inconsistent state #2882
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
bzoz
pushed a commit
that referenced
this pull request
Jun 15, 2020
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. |
musm
pushed a commit
to musm/libuv
that referenced
this pull request
Jul 23, 2020
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)
musm
pushed a commit
to musm/libuv
that referenced
this pull request
Jul 23, 2020
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)
JeffroMF
pushed a commit
to JeffroMF/libuv
that referenced
this pull request
May 16, 2022
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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