fix(nvhttp): non-blocking TLS shutdown in SunshineHTTPS destructor - #5617
Open
awerty-noob wants to merge 1 commit into
Open
fix(nvhttp): non-blocking TLS shutdown in SunshineHTTPS destructor#5617awerty-noob wants to merge 1 commit into
awerty-noob wants to merge 1 commit into
Conversation
The ~SunshineHTTPS() destructor performs a synchronous ssl::stream::shutdown() on the single io thread of the HTTPS server (port 47984). After the local close_notify has been written, SSL_shutdown() waits for the peer's close_notify using a blocking read_some()->poll(fd, POLLIN, -1) on the underlying socket. Clients that keep the connection open after receiving a response - e.g. the Moonlight HTTP connection pool (Android app, Moonlight-Switch, Moonlight-N3DS) - never send a close_notify, so the io thread blocks forever and the entire HTTPS server stops accepting connections. Symptom: 'Host reachable, but app list authentication fails, HTTP request timed out' plus a growing listen backlog on port 47984, while 47989/RTSP keep working. Set the socket non-blocking before shutdown so the attempt returns immediately with would_block instead of hanging; the close_notify is still sent best-effort and the socket is closed right after.
2 tasks
|
Member
|
Thank you for the PR submission, but it looks like you used AI to create this PR. Please read and follow our Contributing guidelines and specifically our AI Usage policy. Additionally, please update the PR to use the correct template. You can find it at https://github.com/LizardByte/.github/blob/master/.github/pull_request_template.md?plain=1 |
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
Fixes the HTTPS server (port 47984) permanently hanging after a Moonlight/Host client connects, as reported in #5289: "Sunshine connections fail after using certain Moonlight clients".
Root cause
nvhttp::SunshineHTTPSwrapsSimpleWeb::HTTPS(anasio::ssl::stream<tcp::socket>) and added a destructor that performs a synchronous TLS shutdown:The
SunshineHTTPSconnection object is destroyed on the single io thread of the HTTPS server (Simple-Web-Server runs withthread_pool_size = 1, i.e. one thread serves all connections on 47984). When the connection is destroyed (e.g. right after a response is written withclose_connection_after_response),ssl::stream::shutdown():close_notify, thenclose_notify— via a blockingread_some()→socket_ops::poll_read(fd, -1)because the socket is blocking.Clients that keep the connection open after receiving a response — the Moonlight HTTP connection pool (Android app, Moonlight-Switch, Moonlight-N3DS, …) — never send a
close_notify. The io thread then blocks inpoll()forever, so the server stops accepting all new connections: the listen backlog grows, every new HTTPS request times out (Moonlight: "Host reachable but app list authentication failed, HTTP request timed out"), while port 47989 / RTSP / streaming keep working and the process looks healthy.Confirmed from a core dump of the hung thread:
Fix
Mark the underlying socket non-blocking before
shutdown()in the destructor. TheSSL_shutdown()close_notify is still written best-effort, but instead of blocking forever waiting for the peer reply, the shutdown attempt returns immediately withwould_blockand the socket is closed. The io thread is never blocked, so the server keeps accepting connections.Verification
GET /serverinfothen holds the connection open (no FIN, no close_notify — exactly the pooled-connection pattern) wedges the server; a probe 25 s later times out.Note
The
SunshineHTTPSwrapper was introduced inf048510e("fix(nvhttp): wrap TLS socket to ensure graceful closure", #3077) which is newer than v0.23.1 — matching the issue's report that this affects all versions newer than v0.23.1.