Retry listener select() on EINTR instead of exiting the listener thread - #733
Closed
dmitriy-bty wants to merge 1 commit into
Closed
Retry listener select() on EINTR instead of exiting the listener thread#733dmitriy-bty wants to merge 1 commit into
dmitriy-bty wants to merge 1 commit into
Conversation
The background listener thread (listenerRun) treated any select() failure, including EINTR, as fatal: it logged an error and returned, terminating the thread permanently. select() is not restarted by the kernel after a signal (even with SA_RESTART, see signal(7)), so a single stray signal delivered to the listener thread killed the accept loop while the listening sockets stayed open. New clients then completed the TCP handshake but were never accept()ed and hung forever. Retry the loop when errno == EINTR; preserve the existing log-and-return behavior for all other select() errors. Closes LibVNC#732
Member
|
#734 was more complete, merged that one. |
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
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.
Summary
The background listener thread (
listenerRuninsrc/libvncserver/main.c),used when the server runs with
rfbRunEventLoop(screen, usec, TRUE), exitspermanently the first time
select()fails withEINTR. Once it exits, thelistening sockets stay open but nothing ever calls
accept(), so new clientscomplete the TCP handshake and then hang forever. This fixes #732.
Root cause
listenerRuntreats anyselect()error as fatal:EINTRis not a real failure — it just means a signal was delivered while thethread was blocked in
select(). Persignal(7),select()is one of thecalls that is not automatically restarted by the kernel after a signal
handler runs, even when the handler is installed with
SA_RESTART. So a singlestray signal landing on the listener thread makes
select()return-1witherrno == EINTR, the threadreturns, and the accept loop is gone. Thelistening file descriptors are never closed, so from a client's point of view
the port is open and
connect()succeeds, but the connection is never acceptedand the viewer waits indefinitely.
Change
Guard the error path with an
EINTRcheck: on an interruptedselect(),continuethe loop and re-enterselect()instead of logging an error andreturning. All other
errnovalues keep the existing log-and-return behavior,so genuine failures are still surfaced exactly as before.
This is the standard EINTR-retry idiom.
errno.his already included in thisfile, and the change is scoped to the single reported site in
src/libvncserver/main.c.Validation
cmake -S . -B build && cmake --build build --target vncserver), pthreads threading backend, no new warnings.rfbRunEventLoop(screen, usec, TRUE), lets the listener thread reachselect(), then deliversSIGUSR1to that thread viapthread_kill()toforce
EINTR, and finally opens a TCP connection and waits for the RFBProtocolVersion greeting the server sends on
accept():listenerRun: error in select: Interrupted system callislogged, the listener thread dies, no greeting arrives, the client read
times out.
and sends
RFB 003.008; the connection succeeds.Related (not changed here)
The per-client processing loop in
clientInput(same file) has the same shape —select()returning< 0breaks the loop, terminating that client's thread,without an
EINTRguard. That path only tears down one already-connectedclient rather than the whole accept loop, so it is a separate, lower-severity
issue; I have kept this PR scoped to the listener thread reported in #732 and
am happy to follow up on the client loop separately if you'd like.
Closes #732