Repository navigation
Conversation
readSocket() read at most 1023 bytes per call and left message reassembly to a FIXME. The socket is a SOCK_STREAM and carries no message boundaries, so any reply longer than the buffer -- or merely split across two reads by the kernel -- was parsed as two independent messages. Both halves then failed to parse, the original reply was lost, and the waiting open() sat out its full backoff. Worse, the effect is self-sustaining: once the stream desynchronises every subsequent message on the connection is misparsed, so a single long reply broke hydration until restart. A V2/HYDRATE_FILE_RESULT carrying a path plus a free-form arguments.error string clears 1 KB without trying. processSocketInput() now drains the socket into a persistent buffer, dispatches only complete newline terminated messages, and keeps the remainder for the next read. handleReceivedMsg() correspondingly handles a single message and no longer splits on newlines itself, which also retires its trailing-empty-fragment case. The buffer is bounded so a peer that never sends a newline cannot grow it without limit.
First, the job was registered by the socket thread only after the send succeeded. The job is now inserted before the message is posted, so no waiter can observe the absence of a job it just posted. Second, SharedMap now carries a condition variable and SharedMap::waitForJob() blocks on it with a deadline. Also made _transfer_id atomic.
The bugs fixed in this branch are timing and framing dependent, which is exactly the kind that comes back unnoticed. The test stands in for the desktop client on a real AF_UNIX socket and drives the actual SocketThread and SharedMap. It covers a message split across two writes, a reply far larger than any single read buffer, several replies batched into one write, the stream still being in sync afterwards, a silent client timing out within its deadline, a late reply being observed without a growing backoff, and PostMsg reporting a message dropped during shutdown. Verified to fail against the pre-fix framing code. The repository had ctest wired up but no tests, so this also gives the existing "Run tests" CI step something to run.
TheOneRing
reviewed
Oct 6, 2026
Comment on lines
+198
to
+200
| // EAGAIN on the non-blocking socket simply means there is nothing more | ||
| // to read right now. EWOULDBLOCK is an alias for it on Linux and macOS, | ||
| // but is checked separately for portability to platforms where it isn't. |
Collaborator
There was a problem hiding this comment.
That doesn't read like a serious issue. But we abort the read afterwards with a comment that it is serious?
Member
Author
There was a problem hiding this comment.
It's a != operator... we abort for all errors that are not the (unserious) EAGAIN
TheOneRing
reviewed
Oct 6, 2026
| if (!_rxBuffer.empty()) { | ||
| std::cerr << "Socket closed with " << _rxBuffer.size() << " bytes of incomplete message, discarding" << std::endl; | ||
| _rxBuffer.clear(); | ||
| } |
Collaborator
There was a problem hiding this comment.
0 means eof but doesn't that just indicate that there is nothing more to read right now?
In which case we must not return bud parse the output?
TheOneRing
reviewed
Oct 6, 2026
| size_t pos; | ||
| while ((pos = _rxBuffer.find('\n')) != std::string::npos) { | ||
| handleReceivedMsg(_rxBuffer.substr(0, pos)); | ||
| _rxBuffer.erase(0, pos + 1); |
Collaborator
There was a problem hiding this comment.
we can move the erase to after the loop so we have to resize only once
Merged
Member
Author
|
Sorry, I screwed up this one - continued in #73 |
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.
Continued PR #61