Repository navigation
Fix peer EOF and destroy - #4
Merged
julian-CStack merged 20 commits intoOct 8, 2026
Merged
Conversation
sneurlax
force-pushed
the
fix/peer-eof-and-destroy
branch
from
October 7, 2026 17:45
b2c9282 to
68ab7ff
Compare
TunnelServer accepts any number of clients, hands back each tunnel end in order, and can park a handshake at the greeting, at the CONNECT reply, or part-way through the server's TLS flight. It can also stop accepting and fill its backlog so a later TCP connect stays pending. peer_eof_test uses it instead of its own proxy.
sneurlax
force-pushed
the
fix/peer-eof-and-destroy
branch
2 times, most recently
from
October 7, 2026 21:12
53a026f to
4a00101
Compare
…error dart:io can report a failed read or write through the socket's error handler before the call returns, then close the socket. A plain socket does so for a read that fails, which is how macOS surfaces a reset by the peer (Linux usually raises an error event first, which the channel already handled), and a secure socket does so for a write after the connection closed. RawChannel checked for a closed channel only before each read or write, so after such a failure it created a waiter that no event would ever complete, and a SOCKS handshake cut short by a reset waited for its deadline. Check again after the call, before waiting. SocksConnection reads through the same channel and is fixed too.
reconnect() closed the old connection, then reset the close state and dialled again. A close() during that window shared the old close and was then forgotten: the reconnect finished and the socket ended connected. Record that a close was requested and stop the reconnect at its next step. The proxy TCP connect is a cancellable ConnectionTask, so close() ends it too, and a connect that fails after close was requested reports the cancellation rather than its own error. The task exists only a few microtasks after the old connection's close completes; a close() requested in that gap, for instance from the old inputStream's done event, is applied to the task as soon as it exists. When the raw socket closes mid-handshake with part of the server's flight still buffered, dart:io's secure socket never fails the pending handshake, so connectTo() waited for the handshake deadline, thirty seconds by default. Complete the cancel signal from close() as cancel() does, so the handshake ends at once. Also document that reconnect() works after cancel() once a target is known; the instance is not spent.
sneurlax
force-pushed
the
fix/peer-eof-and-destroy
branch
from
October 7, 2026 21:28
4a00101 to
6f61e96
Compare
Destroying the underlying socket mid-write could report the write as sent: dart:io completes a pending flush on a destroyed Socket normally, so outputStream.addStream() and write() succeeded although nothing was delivered. SOCKSSocket.destroy() fails pending writes first, then tears the connection down without draining output, and _queueWrite() no longer trusts a flush that completes after the connection has failed. destroy() also ends a connect, TLS handshake or reconnect() in flight with SocksCancelledException, and a later close() completes normally even when destroy() failed a close that was already draining. It may be called from an upload source's onCancel: the sink clears its bound stream before cancelling it, so the re-entrant teardown completes the stream's future once. Fixes #2
When the server half-closed the tunnel, SOCKSSocket closed itself, so every write started afterwards failed with "Output sink is closed", although the server was still reading. create(closeOnPeerEof: false) now ends only inputStream on peer EOF; the socket stays connected and keeps writing until close(). The default keeps the 1.4.0 behaviour, so callers who hit #3 must opt in. See #3
Release as 2.0.0 and document migration to managed I/O, teardown, and TLS. Remove close-time flush retries for operations that callers can no longer start outside the wrapper. Tests use the wrapper APIs instead of the removed getter. BREAKING CHANGE: remove SOCKSSocket.socket; use the wrapper APIs and built-in TLS instead.
SOCKSSocket tracked its life in a dozen flags that each held part of the same answer: _state, _closing, _cancelled, _greetingStarted, _greetingComplete, _requestStarted, _peerReadClosed, _sslUpgraded, _nativeSocketOpen, _nativeSocketNeedsDestroy, _applicationInputPaused, plus a generation counter to ignore stale callbacks. Every teardown race found in review was two of them disagreeing. Replace them with a private phase enum (idle, greeting, greeted, requesting, connected, closing, cancelled, closed, failed) and derive the public state from it, so the API is unchanged: idle, closing, cancelled, closed -> disconnected greeting, greeted, requesting -> connecting connected -> connected failed -> error Two simplifications make that possible. The plain connection now runs over the same raw channel as TLS and SocksConnection, which removes the native-socket bookkeeping, the second response controller and channel, and the generation counter (callbacks compare the transport or sink they belong to instead). Handshake replies are read straight from the channel in exact sizes, as SocksConnection already does, which removes the handshake controller, the reply accumulator and the buffer that preserved application bytes after the CONNECT reply; those bytes now simply stay in the socket until inputStream starts. A greeting or authentication reply with trailing bytes still fails the handshake. All teardown paths go through two primitives: _abandon destroys the transport and ends inputStream, _fail also records the failure for the output sink. A closed socket stays closed; failures its own teardown provokes do not move it to error. Observable differences: state is disconnected as soon as close() starts draining rather than once it completes, and transport errors read the same on plain and TLS connections. The class is 290 lines shorter and all 202 tests pass unchanged.
It has been deprecated since 1.2.0 because it does not await the proxy connection, so connect() raced _init(). Every consumer uses create(). BREAKING CHANGE: remove SOCKSSocket(); use SOCKSSocket.create().
They exposed the broadcast controller and the transport subscription behind inputStream. Closing the controller or cancelling the subscription from outside skipped the lifecycle tracking that 2.0.0 centralises, for the same reason the socket getter was removed. Nothing outside this package's own tests used them; those tests now check the delivered data rather than the subscription's pause state.
sneurlax
force-pushed
the
fix/peer-eof-and-destroy
branch
from
October 7, 2026 22:10
6f61e96 to
34ec799
Compare
The badge block was indented, so it rendered as code; the license badge pointed at another repository and its link was unterminated; the snippets used Tor.instance without saying where it comes from.
The timeout message printed whole seconds, so a 500 ms deadline read "timed out after 0 seconds".
The Flutter app imported the library file that defines ConnectionState beside Flutter's own, which socks.dart exists to avoid. The tor_ffi_plugin git dependency had no ref, so builds were not reproducible. The example README was Flutter boilerplate with a TODO.
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.
Closes #2 and see #3