add: TcpServer::sendAsync — a send that does not block the caller - #215
Merged
Conversation
send() held clientsMutex_ across its blocking send loop. That mutex also guards client registration, removeClient, disconnectClient and every other send, so a single peer that stopped reading froze the server: no new client could be accepted, no client could be disconnected, and no other send could proceed (head-of-line blocking). TcpClient already had a dedicated sendMutex_ and did not suffer this, which made the server side an asymmetry rather than a design. Each client now owns an internal::TcpSendChannel — its own send mutex plus the socket and an `open` flag — held by shared_ptr so an in-flight send keeps the channel alive after the client has been erased from the map. send() takes clientsMutex_ only long enough to look the channel up. Releasing the map lock during the send means a concurrent teardown could close the descriptor mid-write, so closeChannel() fixes the order: clear `open`, shutdown() to wake a send already parked in the kernel, and only then take the send mutex and close. Without the shutdown the close would itself wait behind the stalled send. Also protect against SIGPIPE, whose default action terminates the process on a write to a socket the peer already closed: MSG_NOSIGNAL where it exists, and SO_NOSIGPIPE on each accepted socket for Apple, which has no such flag. The process only survived this before by accident, via httplib's process-wide signal(SIGPIPE, SIG_IGN) when an HTTP server happened to be running. Add setSendTimeout() as the SO_SNDTIMEO knob. It defaults to 0 (block until sent) so existing behaviour is unchanged — a finite default would silently break senders pushing large payloads over slow links. Document the semantics that were missing from the reference: send() and broadcast() are blocking, broadcast() sends one client at a time, and setSendTimeout() bounds the wait. TcpClient::connect already said "(blocking)" while TcpServer::send said nothing, so reading it as asynchronous was fair. core/tests/tcpServerSend asserts the invariant: with 8 MB parked in a send to a non-reading peer, getClientIds(), a fresh accept and a send to a healthy client all still complete. Verified in both directions on Linux — the test passes with this change and fails (exit 1, 8.4s, no hang) against the previous code.
…on web Three follow-ups to the per-client send channel, from CI and from verification on macOS and Windows. The SO_SNDTIMEO branch used struct timeval without including <sys/time.h>. Linux, macOS, Android and iOS pull it in transitively; emscripten does not, so the web build failed with an incomplete type. Include it explicitly, as tcUdpSocket.cpp already does. send() fired notifyError() while still holding the channel's send mutex. onError listeners run inline on the sending thread by default, and the obvious handler to write is one that disconnects the offending client — which re-enters that same non-recursive mutex through closeChannel() and wedges the sender. The failure is now recorded and reported after the lock is released. This deadlock predates the per-client channel (it was the same shape on clientsMutex_), but setSendTimeout() turns send failures from an accident into a routine path, so it becomes much easier to hit. A send timeout that fires mid-payload left the connection open with a truncated message on the wire, so the next send() spliced a fresh message onto a partial one — reproduced on macOS: 8 MB timed out after 678428 bytes, and a following 29-byte send arrived immediately after them. Drop the client instead. A timeout that wrote nothing leaves the stream intact and does not force the drop. Correct the SIGPIPE comment: current Apple SDKs do define MSG_NOSIGNAL (sys/socket.h, 0x80000 on macOS 26.5), so TC_SEND_FLAGS is not 0 there. A four-way probe confirmed each mechanism suppresses the signal on its own, so SO_NOSIGPIPE stays for older SDKs. Windows defines neither and needs neither. The test no longer assumes a fixed payload size can wedge a send. Winsock returns as soon as the payload reaches its own send buffer, so 8 MB was accepted outright and the premise silently failed on Windows. Send in chunks and watch for progress to stop instead; if the platform still will not park a send, print SKIP for that precondition and keep checking the invariant that matters. Also set SO_RCVBUF before connect(), since the size takes part in the window negotiation and does nothing once the socket is established.
c6d04c3 committed the test and the reference prose but not tcTcpServer.cpp itself — the working tree copy was still sitting in a stash at commit time, so the commit message described changes that were not in it. This carries the actual source edits: <sys/time.h> for the SO_SNDTIMEO timeval on emscripten, notifyError() moved outside the send lock, the mid-payload timeout dropping the client, and the corrected SIGPIPE comment.
Accepted sockets are now non-blocking and both the send loop and the receive thread wait in 100 ms slices. A blocking send cannot be interrupted portably: Winsock's shutdown() does not wake a send() that is already parked, so closeChannel() waited on the send mutex until the peer moved and disconnectClient() hung (windows-latest, run 33227844653: "disconnecting the stalled client unblocks its send" FAIL). Clearing `open` now cuts a parked send short within one slice, on every platform. SO_SNDTIMEO goes away with it — the send loop measures its own deadline, so the timeout applies to every send rather than only to clients accepted after setSendTimeout(). The receive thread polls for the same reason: a non-blocking recv() would otherwise spin, and it now notices the server stopping without waiting for traffic that may never arrive.
setSendTimeout() is the one public method this branch adds, and it was missing from both the Lua usertype and the FOR_AI_ASSISTANT API index. Hand-applied rather than regenerated: docs/reference/generate.js reads the AST of the host it runs on, so regenerating here (linux) drops the macOS-only VideoWriter::lockFrame/submitFrame that sit inside #if TC_ASYNC_SCREEN_CAPTURE — the binding set diffs to exactly "+setSendTimeout, -submitFrame" plus shard re-packing. These two lines are what a macOS regeneration emits; the next full regen there reconciles the rest. Compile witness: addons/tcxLua/exampleSimple builds clean against it.
tcpServerSend decided the send was wedged only once a 1 MB chunk had completed and progress then stopped (`now > 0 && quietRounds >= 4`). On macOS the first chunk never completes -- against a peer with a 4 KB receive buffer the send blocks inside it -- so chunksSent stayed 0, the premise was reported as SKIP, and "the parked send returned after disconnect" was silently skipped behind the same `parked` guard. The run also burned its full 10 s detection window every time. Silence plus a send thread that has not returned is what says "parked"; a chunk counter that never leaves 0 says it just as well as one that stalls at 40. Linux and Windows reach the same state through the other shape (buffer, then stop) and are unaffected -- both still pass 13/13. Found by re-running the suite on macOS 26.5.
…60 s setSendTimeout() promised to guard against "a peer that has stopped reading", but the implementation timed the whole call: a healthy peer on a slow link was dropped for the offence of being sent a large payload, because the deadline never moved once the send began. It now restarts every time bytes are accepted, so what it measures is silence. That is also what makes a non-zero default safe. A total-elapsed timeout has no defensible default -- the right number depends on payload size and link speed, and being wrong disconnects a working client mid-payload. An idle timeout does: no healthy peer goes 60 seconds without accepting a single byte, and a peer that does cannot be distinguished from a dead one. The default therefore moves from 0 (wait forever) to 60 s, turning an un-debuggable hang into a reported failure. Pass 0 to keep waiting indefinitely. The regression test sends 16 MB to a peer draining 256 KB every 40 ms -- 2.05 s of transfer against a 1 s timeout, never 40 ms idle. Against the previous code it fails at 1.05 s. 16 MB because a smaller payload is absorbed by the sender's socket buffer outright and never waits on the peer at all; the peer keeps its default receive buffer for the same reason, since the 4 KB one the other cases use caps every recv() at a few KB and would throttle the drain loop below the rate being modelled. The check reports SKIP if a platform still swallows the payload faster than the timeout, rather than passing without exercising anything.
The idle-timeout case reported SKIP on Windows: against a default-sized receive window Winsock took the whole 16 MB payload into its own send buffer in 50 ms, so the send never waited on the peer and the check passed over the invariant instead of exercising it -- the same shape as the macOS SKIP fixed in 5a079f9, on the other platform. Cap the peer's receive window at 64 KB. That bounds what any sender can push before it has to wait, and it also caps what a single recv() returns, so the drain interval rather than the sink size is what sets the rate: 64 KB every 10 ms, about 6 MB/s. 16 MB still takes a couple of seconds against a 1 s timeout, and the gaps stay two orders of magnitude clear of it. The payload stays at 16 MB. 4 MB was tried with the smaller window and went the other way -- it fits Linux's send buffer outright, so that platform stopped waiting and started reporting SKIP instead.
…o wait on" This reverts commit 0aaf8af.
Winsock's send buffering is not bounded by the peer's advertised window, so the SKIP cannot be tuned away from the test side: measured, shrinking that window makes it worse (16 MB absorbed in 50 ms against a default window, 10 ms against a 64 KB one) while macOS slows from 8.4 s to 13.7 s, because the small window also caps what each recv() returns. The only lever is SO_SNDBUF on the server's own socket, which is not the test's to set. The SKIP is sound rather than a gap: what this case guards is that our own deadline restarts on progress, which is arithmetic inside send() and not platform behaviour, and both macOS and Linux reach the state and check it. Recorded at the check so the next reader does not repeat the experiment.
disconnectClient() detached the client's receive thread and erased it from clientThreads_, which also emptied the map that disconnectAllClients() then walked looking for threads to join -- so stop() joined nothing, returned while every client thread was still running, and ~TcpServer() went on to free the object those threads were still reading members of (running_ on each pass of their loop, clientsMutex_ and the event objects on the way out). A narrow window, and undefined behaviour. Move the thread out under the lock and join it after releasing it: the thread being joined takes that same lock on its way out through removeClient(), so joining while holding it would deadlock the pair. The caller's own thread is the one case that cannot be joined -- an onReceive listener disconnecting its own client runs on it -- so that one is still detached, and it is already unwinding. This predates the branch; it is here because the branch is where TcpServer's threading is being made correct. Two checks. An onReceive listener runs ON the client's receive thread, so one that is still busy when stop() is called holds that thread: stop() now takes 598 ms against a listener sleeping 600 ms, where the detaching version took 5 ms -- the bug, observable without a sanitizer. Then twenty rounds of building and destroying a server with live clients, because joining is the fix that fails in the other direction, by hanging.
The waits in send() and in the receive thread used a 100 ms slice so they could re-check whether the channel had been closed. That was defensive rather than necessary: Winsock's refusal to wake a parked send() is what forced the waiting out of send() and into select()/poll() in the first place, and a socket that closeChannel() has shut down is reported ready by those immediately. The re-check is still there; it just no longer needs to be reached on a timer. Idle cost was one wakeup per client per 100 ms -- 10 per second per connection, whether or not anything was happening, which at a thousand connections is ten thousand wakeups a second to learn nothing. 10 s rather than a tuned-looking number: this timeout should never be what ends a wait, so it is set far past any latency it could plausibly explain. A wait that reaches it is a bug, not a setting. The disconnect case now prints what it measured. On Linux disconnectClient returns in 5 ms against the 10 s backstop, so the wakeup is the shutdown and not the timeout; macOS and Windows report their own numbers in CI.
Two contention points that only show up once there are many clients. The registry mutex was exclusive, but most of what takes it only reads: every send() looks its channel up through findChannel(), and a draw loop calling getClientCount() does the same. Those took turns with each other for no reason. It becomes a shared_mutex — readers (findChannel, getClientCount, getClientIds, getClient, and the receive thread's initial socket lookup) share it, and only registering, removing and disconnecting a client take it exclusively. broadcast() took that lock once for the list of ids and then once more per id, through send() -> findChannel(): a broadcast to a thousand clients locked the registry a thousand and one times, and did it from a draw loop. It now takes one snapshot of the channels and sends on those. The send loop moves into sendToChannel() so both paths share it. A client that disconnects mid-broadcast is now reported by its own channel being closed rather than by a lookup that finds nothing, which is the more accurate of the two errors anyway. None of this lifts the ceiling on how many clients a thread-per-client server can hold -- that is the threads, and it is a different design. It removes the reasons this one gets slower before it gets there.
…ect() either The backstop experiment failed, and usefully. Measured across CI: Linux disconnectClient 5 ms shutdown() wakes poll() macOS disconnectClient 35 ms shutdown() wakes poll() Windows disconnectClient 4+ s shutdown() does NOT wake select() Winsock declines to wake a waiting select() for the same reason it declines to wake a parked send(). On Windows the slice is therefore not a backstop that should never be reached -- it is the mechanism by which a disconnect is noticed at all. macOS failed differently and is worth keeping in view: every round of the teardown stress case took exactly one slice, which points at a receive thread waiting on a descriptor that was closed and recycled under it, through the window between its open check and its wait. The short slice bounds that to 100 ms; the long one turned it into 10 s a round. Both are fixable -- wake Windows through an event the disconnect signals, and stop closing a descriptor a receive thread may still be waiting on by joining that thread before the close rather than after. Neither is a one-line change, so the measurements go in the source where the next attempt will find them. The idle cost this was meant to remove is unchanged: 10 wakeups per second per client.
… caller send() returns only once the whole payload has reached the kernel, so calling it from a draw loop against a peer that has stopped reading stalls the frame. fix/tcp-send stopped one such peer from freezing the whole server, but the caller still waited. This is the part that was still open. Each client now has a send queue and a writer thread behind TcpSendChannel. sendAsync() copies (or takes) the payload, queues it and returns; every non-zero SendResult::id completes exactly once through onSendComplete, including payloads still queued when the client is disconnected or the server stops. Back-pressure is a high-water mark rather than a cap, so a single message of any size still goes through on an empty queue and there is no separate "too large" case; a full queue is refused with QueueFull and does not drop anyone. send() becomes sendAsync() plus a wait on that one id, which keeps sync and async sends to a client in order and leaves one code path and one idle timeout. It lends its buffer to the queue instead of copying, so it stays as cheap as it was. broadcast() now queues to everyone before waiting, so a slow peer no longer delays the clients behind it. The writer thread is also the only thread that touches the descriptor, and it is the one that closes it on its way out. Teardown shuts the socket down, joins the writer and only then lets the descriptor go, so a close can no longer be raced against a send in flight. Measured against a peer that never reads: a 4 MB sendAsync returns in 5.4 ms (the copy), where send() waits for the idle timeout. The regression test grows from 19 checks to 36, covering completion accounting, the high-water mark, and sync/async ordering.
A server streams a 2 MB payload every frame to a client in the same process and graphs the frame time. Press A and the draw loop hands each payload to a queue and carries on; press B and it waits for every one of them, which is the same stall the Discord question was about. The queued-bytes bar against the high-water mark is what back-pressure looks like from the outside. Exercises the whole new surface: sendAsync, onSendComplete, broadcastAsync via the counters, setSendAsyncBufferSize (the [ and ] keys) and getSendAsyncPendingBytes.
Prose for every new public symbol in api-reference.toml (en/ja/ko), which the 100%-documented gate requires, plus refreshed wording for send() and broadcast() now that both go through the queue. FOR_AI index regenerated. luagen and luagen-types both let an rvalue-reference parameter through: nothing in the API had one until sendAsync(int, vector<char>&&), and sol2 cannot form a callable from a lambda taking T&&, so the generated bindings stopped compiling. Lua has no move semantics, so such a parameter is not bindable and the copying overload is what should be bound — sendAsync(id, string) in this case.
…latform The teardown case asserted that every queued send comes back as Disconnected, which assumed no part of a 4 MB payload could reach a peer that never reads. That holds on Linux and not on Windows: Winsock accepts a multi-megabyte payload whole — it locks the caller's pages and sends in the background rather than copying into a socket buffer the peer's window bounds — so the first payload is written in full and correctly completes as a success. Measured on Windows 11 / MSVC: payload 0 accepted by a single send() in 0.3 ms, payloads 1-3 blocked. The check failed there on a premise, not on the behaviour it guards. Pair each completion with its own bytesSent instead: whatever got out whole must report success, whatever did not must report Disconnected. That is the actual invariant, it holds on every platform, and it covers bytesSent, which nothing checked before. The count of unwritten sends is printed rather than asserted.
The example exists to show the difference between two keys, which until now needed a person to press them — so the one thing it demonstrates could not be checked automatically on any platform. registerControlTools() lets A and B be injected and the frame-time graph read back. It costs nothing when unused: the MCP server only starts when TRUSSC_MCP=1 is in the environment, so no port is opened otherwise and there is nothing to collide with the TCP server this example runs on 9002. Verified both ways. Same opt-in four other examples already use.
The example demonstrated nothing on Windows: A and B both held 60 fps and looked identical. The reason is not the implementation but the premise — a peer that drains as fast as it is fed never makes send() wait, and Winsock does not make it wait even then, since it accepts a large payload whole rather than filling a socket buffer the peer's window bounds. S now holds the client's receive listener for 40 ms per chunk, which is what a consumer falling behind looks like from the sender's side. That gives both modes something to wait for on every platform. It also brings the queue UI to life: refused (queue full) had never once fired, because the queue never got near its mark. Measured here, streaming 2 MB per frame: receiver keeping up pending 2 MB refused 0 128 MB through receiver slowed pending 16 MB refused 38 24 MB through The 16 MB is the high-water mark, pinned — which is the picture back-pressure was supposed to draw and previously never did.
Regenerating the bindings on Linux emitted two lambdas with the same signature into createWindow's sol::overload — one taking `settings`, one taking `a0`. tcWindow.h declares createWindow twice: once for desktop with the parameter named, and once as an inline fallback for everything else with it unnamed, and the Linux AST walk picked up both. sol2 takes the first match so nothing misbehaved, but the second lambda is noise that upstream does not have. Confirmed against a regeneration on macOS, which emits one: that run is byte-identical to what is committed here everywhere else — the usertype shards, the FOR_AI index and the openFrameworks mapping all match, and the set of names bound from Lua is the same on both hosts.
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
1 participant
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 #214.
A user asked on Discord whether
TcpServerdeliberately has no asynchronous send. It did not, and the answer turned out to have two halves.The first half shipped earlier on this branch:
send()held the server-wide client mutex across a blocking send, so one peer that stopped reading froze client registration, disconnects and every other send. That is fixed, and the wait is now bounded by an idle timeout.The caller still waited, though, and that was the actual request. This is the second half.
What changes for someone using it
Nothing, unless they want it.
send(),broadcast()and every existing signature behave as before. What is new:sendAsync(clientId, ...)— queues and returns; three overloads, one of which takes avector<char>&&without copyingbroadcastAsync(...)— returns how many clients accepted it; the payload is buffered once and shared, not copied per peeronSendComplete— fires exactly once for every idsendAsynchanded out, including payloads still queued when the client is disconnected or the server stops (those reportDisconnected)setSendAsyncBufferSize/getSendAsyncBufferSize/getSendAsyncPendingBytes— the back-pressure controlsSendResult,SendError,TcpSendCompleteEventArgs,sendErrorName()in the newtc/network/tcSendResult.h, shared soTcpClient::sendAsynccan land on its own laterHow it works
Each client gets a send queue and a writer thread behind
internal::TcpSendChannel.send()becomessendAsync()plus a wait on that one id. Sharing the queue is what keeps sync and async sends to a client in order, and it leaves one code path and one idle timeout instead of two. It lends its buffer to the queue rather than copying, so it stays as cheap as it was.broadcast()now queues to everyone before waiting, so a slow peer no longer delays the clients behind it.Back-pressure is a high-water mark, not a cap.
sendAsync()is refused withQueueFullonly when the queue already holds the limit, so a single message of any size still goes through on an empty queue and there is no separate "too large" case. A full queue does not disconnect anyone — only the idle timeout does. The 16 MB default is chosen so a frame streamer learns about back-pressure after a frame or two rather than after a large buffer of latency has piled up.The writer thread is the only thread that touches the descriptor, and it is the one that closes it. Teardown shuts the socket down, joins the writer, and only then is the descriptor gone. That removes a hazard the previous round left open: a close racing a send in flight, on a descriptor the kernel is free to recycle.
Measured
Against a peer that never reads, a 4 MB
sendAsyncreturns in well under a millisecond, wheresend()waits for the idle timeout.CI run 33832776033, all nine jobs green:
disconnectClientis under one 100 ms slice everywhere.The regression test grows from 19 checks to 35, covering completion accounting, the high-water mark, sync/async ordering and
bytesSent. One assertion had to be rewritten along the way: it demanded that every queued send come back asDisconnected, which assumes no part of a 4 MB payload can reach a peer that never reads. Winsock accepts such a payload whole — it locks the caller's pages and sends in the background rather than filling a socket buffer the peer's window bounds — so on Windows the first one is genuinely written. Each completion is now paired with its ownbytesSent, which is the real invariant and holds everywhere. The2 of 4above is that difference, and it is printed rather than asserted because it varies with timing.examples/network/tcpAsyncExampleStreams 2 MB per frame and graphs the frame time.
Astreams withsendAsync,Bwithsend,Sslows the receiving end down — which is what makes the two differ at all, since a peer that keeps up never makes either one wait.Verified on Windows hardware with the receiver slowed:
sendAsyncsendIt is drivable over MCP (
registerControlTools(), which opens nothing unlessTRUSSC_MCP=1is set), so that comparison can be checked automatically rather than by hand.Also here
luagenandluagen-typesboth let an rvalue-reference parameter through. Nothing in the API had one untilsendAsync(int, vector<char>&&), and sol2 cannot form a callable from a lambda takingT&&, so the generated bindings stopped compiling. Lua has no move semantics, so such a parameter is not bindable and the copying overload is what should be bound.Prose for every new public symbol is in
api-reference.toml(en/ja/ko) —check.js --strictreports 0 undocumented — and the FOR_AI index and Lua bindings are regenerated.Known, and following separately
A send that times out having written nothing completes as
SendError::DisconnectedwhileonErrorsays "Send timed out" and the client is still connected — so a listener cannot tell "the peer is gone" from "the peer is slow" without parsing the message string. Found by probing with a large number of one-byte sends, which puts a buffer boundary on a message boundary and makes the wrote-nothing case deterministic.The fix is to add
SendError::Timeout. It is additive and compatible, but the enum in #214 was settled deliberately, so it lands in its own PR rather than being slipped in here.Deliberately not here
TcpClient::sendAsync, which the shared types are shaped forSendResult::idandonSendComplete; a callback would need its own threading story that the event already has viaDeliver::Mainselect()on Windows with an event object. The 100 ms slice is the mechanism there rather than a backstop, and the in-source comment records the measurements