fix(server): close socket fd leaks on connection errors#5
Open
missuo wants to merge 1 commit intoMercuryWorkshop:devfrom
Open
fix(server): close socket fd leaks on connection errors#5missuo wants to merge 1 commit intoMercuryWorkshop:devfrom
missuo wants to merge 1 commit intoMercuryWorkshop:devfrom
Conversation
Use socket.destroy() instead of socket.end() to immediately release file descriptors. Add proper error handling and cross-directional cleanup in WSProxyConnection, TCP/UDP sockets, and AsyncWebSocket to prevent resource leaks on abnormal disconnections.
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
This PR fixes multiple socket file descriptor leaks that occur when connections are abnormally terminated.
Background
We discovered this issue while running Asspp (the web version), which uses
@mercuryworkshop/wisp-jsto proxy browser-to-Apple-server TCP connections via WebSocket on the/wisp/path. Every Apple API request from the browser goes through the Wisp proxy.On a VPS with 111GB disk, the disk usage would reach 100% every few days. Investigation using
lsof +L1revealed that thenode dist/index.jsprocess was holding file descriptors to 216.8GB of deleted-but-unreleased files — invisible todubut occupying real disk space reported bydf. These were accumulated socket fds from connections that were never properly closed.After auditing the Asspp codebase, we confirmed its own file operations (chunked downloads, sinf injection, etc.) all have proper cleanup. The leak source was traced to
@mercuryworkshop/wisp-js.Root Causes
1.
NodeTCPSocket.close()usessocket.end()instead ofsocket.destroy()—src/server/net.mjsThis is the primary cause of the fd leak.
socket.end()only half-closes the connection (sends FIN) and waits for the remote side to close as well. If the remote never responds or the connection is in a broken state, the fd is held indefinitely. In a high-throughput proxy scenario, these accumulate rapidly.Fix: Changed to
socket.destroy()for immediate fd release.2.
WSProxyConnectionhas no cross-directional cleanup —src/server/wsproxy.mjsWhen
tcp_to_ws()errors, only the WebSocket is closed — the TCP socket leaks. Whenws_to_tcp()errors, only the TCP socket is closed — the WebSocket leaks. There is no centralized cleanup.Fix: Added a
cleanup()method with aclosedflag to ensure both the TCP socket and WebSocket are always released together, regardless of which direction fails.3. UDP socket error handler doesn't call
socket.close()—src/server/net.mjsThe error handler sets
this.socket = nullwithout callingclose()on the dgram socket, leaking the underlying OS socket resource.Fix: Added
this.socket.close()before nulling the reference.4. TCP
connect()error handler doesn't reject the promise —src/server/net.mjsThe
"error"event handler only logs a warning but never callsreject(). If a connection error occurs, the promise can hang indefinitely, keeping the socket fd alive.Fix: Added
reject(error)when!this.connected.5.
AsyncWebSocket.connect()missingonerrorhandler —src/websocket.mjsNo
onerrorhandler means WebSocket connection errors don't reject the promise, which can hang forever.Fix: Added
onerrorhandler that rejects when not yet connected.6.
create_connectioncatch block doesn't callcleanup()—src/server/http.mjsIf
ServerConnection.setup()throws after starting the pingsetInterval, the catch block only callsws.close()— the interval timer and any partially-created streams are never cleaned up.Fix: Track the
ServerConnectioninstance and callcleanup()in the catch block.Diagnostic Method
Changed Files
src/server/net.mjs—destroy()instead ofend(), TCP error rejects promise, UDP error closes socketsrc/server/wsproxy.mjs— centralizedcleanup()method for bidirectional resource releasesrc/websocket.mjs—onerrorhandler inAsyncWebSocket.connect()src/server/http.mjs— callcleanup()onServerConnectionin catch block