Closed
Conversation
Member
Author
|
CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1459/ EDIT(cjihrig): In case the CI runs get lost to history - both runs were fine. |
Member
Author
|
ping @libuv/collaborators ? |
Member
Author
|
ping? |
Member
Author
|
ping? @libuv/windows |
bnoordhuis
approved these changes
Aug 30, 2019
Member
bnoordhuis
left a comment
There was a problem hiding this comment.
LGTM with some suggestions and a question. Sorry for the delay, I had this on my TBR (To Be Reviewed) list but it fell through the cracks.
src/win/tty.c
Outdated
| if (pSetWinEventHook == NULL || pNtQueryInformationProcess == NULL) | ||
| if (pSetWinEventHook == NULL || pNtQueryInformationProcess == NULL) { | ||
| return 0; | ||
| } |
| LONG idChild, | ||
| DWORD dwEventThread, | ||
| DWORD dwmsEventTime) { | ||
| SetEvent(uv__tty_console_resized); |
Member
There was a problem hiding this comment.
uv__tty_console_resized can be NULL here, I think? Is that bad?
Member
Author
There was a problem hiding this comment.
It cannot, it wont start the watcher if the event could not be created.
Member
|
Can you undo the whitespace changes? The diff is massive now. |
Member
Author
|
I know, tried the "batch commit sugestions" thingy, did not work as intended. |
Continuing improvement of SIGWINCH from PR libuv#2308. Running SetWinEventHook without filtering for the specific PIDs has significant impact on the performance of the entire system. This PR changes the way SIGWINCH is handled. The SetWinEventHook callback now signals a separate thread, uv__tty_console_resize_watcher_thread. This thread calls uv__tty_console_signal_resize() which checks if the console was actually resized. The uv__tty_console_resize_watcher_thread makes sure to not to call the uv__tty_console_signal_resize function more than 30 times per second. The SetWinEventHook will not be installed, if the PID of the conhost.exe process that owns the console window cannot be determinated. This can happen when a 32bit libuv app is running on a 64bit Windows. For such cases PR libuv#1408 is partially reverted - when tty reads WINDOW_BUFFER_SIZE_EVENT, it will also trigger a call to uv__tty_console_signal_resize(). This will also help when the app is running under console emulators. Documentation was alos updated to reflect that. Refs: microsoft/terminal#1811 Refs: microsoft/terminal#410 Refs: libuv#2308
499103f to
b2bdecb
Compare
Member
Author
|
updated and rebased on master |
Member
Author
Member
Author
|
Failures are unrelated. |
bzoz
added a commit
that referenced
this pull request
Sep 5, 2019
Continuing improvement of SIGWINCH from PR #2308. Running SetWinEventHook without filtering for the specific PIDs has significant impact on the performance of the entire system. This PR changes the way SIGWINCH is handled. The SetWinEventHook callback now signals a separate thread, uv__tty_console_resize_watcher_thread. This thread calls uv__tty_console_signal_resize() which checks if the console was actually resized. The uv__tty_console_resize_watcher_thread makes sure to not to call the uv__tty_console_signal_resize function more than 30 times per second. The SetWinEventHook will not be installed, if the PID of the conhost.exe process that owns the console window cannot be determinated. This can happen when a 32bit libuv app is running on a 64bit Windows. For such cases PR #1408 is partially reverted - when tty reads WINDOW_BUFFER_SIZE_EVENT, it will also trigger a call to uv__tty_console_signal_resize(). This will also help when the app is running under console emulators. Documentation was also updated to reflect that. Refs: microsoft/terminal#1811 Refs: microsoft/terminal#410 Refs: #2308 PR-URL: #2381 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Member
Author
|
Landed in 7d950c0 |
vtjnash
added a commit
to vtjnash/libuv
that referenced
this pull request
Aug 2, 2024
The Event should be reset before reading the value, or libuv might miss an update that occurred too rapidly after the previously one. Refs: libuv#2381 Refs: libuv#4485
vtjnash
added a commit
that referenced
this pull request
Aug 5, 2024
liujinye-sys
pushed a commit
to open-vela/apps_system_libuv
that referenced
this pull request
Jul 23, 2025
Continuing improvement of SIGWINCH from PR #2308. Running SetWinEventHook without filtering for the specific PIDs has significant impact on the performance of the entire system. This PR changes the way SIGWINCH is handled. The SetWinEventHook callback now signals a separate thread, uv__tty_console_resize_watcher_thread. This thread calls uv__tty_console_signal_resize() which checks if the console was actually resized. The uv__tty_console_resize_watcher_thread makes sure to not to call the uv__tty_console_signal_resize function more than 30 times per second. The SetWinEventHook will not be installed, if the PID of the conhost.exe process that owns the console window cannot be determinated. This can happen when a 32bit libuv app is running on a 64bit Windows. For such cases PR #1408 is partially reverted - when tty reads WINDOW_BUFFER_SIZE_EVENT, it will also trigger a call to uv__tty_console_signal_resize(). This will also help when the app is running under console emulators. Documentation was also updated to reflect that. Refs: microsoft/terminal#1811 Refs: microsoft/terminal#410 Refs: libuv/libuv#2308 PR-URL: libuv/libuv#2381 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
liujinye-sys
pushed a commit
to open-vela/apps_system_libuv
that referenced
this pull request
Dec 16, 2025
Continuing improvement of SIGWINCH from PR #2308. Running SetWinEventHook without filtering for the specific PIDs has significant impact on the performance of the entire system. This PR changes the way SIGWINCH is handled. The SetWinEventHook callback now signals a separate thread, uv__tty_console_resize_watcher_thread. This thread calls uv__tty_console_signal_resize() which checks if the console was actually resized. The uv__tty_console_resize_watcher_thread makes sure to not to call the uv__tty_console_signal_resize function more than 30 times per second. The SetWinEventHook will not be installed, if the PID of the conhost.exe process that owns the console window cannot be determinated. This can happen when a 32bit libuv app is running on a 64bit Windows. For such cases PR #1408 is partially reverted - when tty reads WINDOW_BUFFER_SIZE_EVENT, it will also trigger a call to uv__tty_console_signal_resize(). This will also help when the app is running under console emulators. Documentation was also updated to reflect that. Refs: microsoft/terminal#1811 Refs: microsoft/terminal#410 Refs: libuv/libuv#2308 PR-URL: libuv/libuv#2381 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Continuing improvement of SIGWINCH from PR #2308.
Running
SetWinEventHookwithout filtering for the specific PIDs has a significant impact on the performance of the entire system. This PR changes the waySIGWINCHis handled.The
SetWinEventHookcallback now signals a separate thread,uv__tty_console_resize_watcher_thread. This thread callsuv__tty_console_signal_resize()which checks if the console was actually resized. Theuv__tty_console_resize_watcher_threadmakes sure to not to call theuv__tty_console_signal_resizefunction more than 30 times per second.The
SetWinEventHookwill not be installed if the PID of theconhost.exeprocess that owns the console window cannot be determined. This can happen when a 32bit libuv app is running on a 64bit Windows.For such cases, PR #1408 is partially reverted - when
ttyreadsWINDOW_BUFFER_SIZE_EVENT, it will also trigger a call touv__tty_console_signal_resize(). This will also help when the app is running under console emulators. Documentation was also updated to reflect that.