Fix win32 select handle leak - #14975
Open
toots wants to merge 2 commits into
Open
Conversation
When the descriptor set is not made exclusively of sockets, Unix.select falls back to a worker thread emulation which registers every socket with WSAEventSelect for the duration of the call. If an error occurs, the sockets are currently left registered and in non-blocking mode, and one event handle per socket is leaked. The test detects the skipped cleanup through Unix.clear_nonblock: ioctlsocket(FIONBIO) fails with WSAEINVAL while a WSAEventSelect registration is active. Its reference file records the current, buggy behaviour, so that the next commit shows it flipping to a successful cleanup.
toots
force-pushed
the
fix-win32-select-handle-leak
branch
from
July 31, 2026 00:49
b041f75 to
1092bd9
Compare
toots
changed the title
In socket_poll, the cleanup loop releasing the event handles, cancelling the WSAEventSelect associations and restoring the blocking mode of the sockets was guarded by nError == 0, so any error during setup or during the wait skipped it altogether. This leaked one event handle per socket and left the sockets registered with a destroyed event and in non-blocking mode. Cleanup is now unconditional; only the collection of the results remains conditioned on the absence of error. A CreateEvent failure is also reported instead of being silently turned into a WAIT_FAILED. The event array was additionally one element too small: it holds one event per query plus hStop, and the number of queries per worker is allowed to reach MAXIMUM_SELECT_OBJECTS. The reference of the test added in the previous commit flips from the skipped cleanup to a successful one.
toots
force-pushed
the
fix-win32-select-handle-leak
branch
from
July 31, 2026 01:11
1092bd9 to
c8da08e
Compare
Contributor
For a more direct examination of the bug you could try using the debug runtime ( |
Contributor
Author
How do you think this could be used? |
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
3 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.
This is a LLM-driven bug reported initially by liquidsoap users at savonet/liquidsoap#5290 and confirmed on the code base.
The commits are split in two:
winelocally which does not implementGetProcessHandleCountso the test was switched to trigger on the socket not being restored to non-blocking, which is one of the process applied when the socket is properly handled when exiting the select loop. A more rigorous test demonstrating the socket leak can also be implemented but I will not be able to run it locally.OK.PS: this message is written by a reguler person (myself!)