fix(ci): stop the macOS editor-ON job hanging in the local socket tests by Mosch0512 · Pull Request #92 · Mosch0512/MuMain

Mosch0512 · 2026-09-29T19:18:25Z

Problem

The macOS arm64 Release, editor ON job of Fork CI has been cancelled on every PR (for example this run). The build passes; ctest then hangs in Local socket bounds the unterminated tail, not a pipelined batch [core][local-socket] until GitHub cancels the job after six hours.

The test's client sends 16 KiB chunks from the same thread that reads them, and the client was blocking. macOS gives an AF_UNIX stream an 8 KiB buffer (net.local.stream.sendspace), so send() waited for a read that could never start. Linux and Windows buffer more than one chunk, so they passed. The socket tests are only built with ENABLE_CONTROL_SOCKET, which is why the editor-OFF jobs never hit this. The transport's behaviour is unchanged; only the test client was wrong.

Upstream: sven-n#660 (closes sven-n#633).

Changes

The hang

The large-payload cases write through SendWhileServing, which sends what the socket takes and lets the connection read before sending the rest. It stops once the client has not got a byte out for five seconds, so a connection that stops reading fails the case instead of spinning.
Only ConnectForLargeWrites makes a client SendWhileServing accepts: non-blocking, so the mode cannot be forgotten, and with a 4 KiB send buffer, so Linux and Windows take the partial sends that only macOS took before.
The final line counts wait for the end of the stream with a deadline instead of assuming the last chunk is readable the moment send() returns.

Test coverage

The pacing case now reaches the read pause: it sends without serving until the writer is held off, checks that the buffered lines reached the pause mark and stopped within one read of it, then serves the rest and checks that every line arrives. With the pause disabled in the transport, the case fails.

Shared code

Core/Platform/NonBlockingSocket.h holds Enable and WouldBlock, used by both LocalSocket.cpp and the test. The test previously had its own copies, and its WouldBlock missed EAGAIN and EINTR.

Time limits

mu_add_test gives every discovered test case a TIMEOUT of MU_TEST_CASE_TIMEOUT_SECONDS (120 s; the slowest case takes about a second), so the limit applies in ci.yml, locally and in the IDE.
Each Fork CI job has timeout-minutes: 90 (the slowest, Windows, takes about 35 minutes). That also covers steps a per-test limit does not: doctest's test discovery during the build, restore and configure.

Testing

Reproduced the hang on Linux (WSL, Ubuntu 24.04) by shrinking the test client's SO_SNDBUF to macOS's 8 KiB. All 9 local-socket cases pass there, with the default buffer and on Windows (MSVC).
All Fork CI jobs pass, including macOS arm64 editor ON (each local-socket case about 0.01 s).
Removing the read pause from the transport makes the pacing case fail.
In a small CMake project using the vendored doctest module, a hanging case with the TIMEOUT property is stopped and reported by name.
clang-format 21.1.8 on the changed lines and cppcheck with the Quality Gates flags are clean.

The pipelined-batch and pacing cases send 16 KiB chunks from the thread that also reads them. The client was blocking, so a chunk larger than the kernel's socket buffer waited for a read that could not start. Linux and Windows buffer more than a chunk; macOS gives an AF_UNIX stream 8 KiB, so the macOS editor-ON job hung in the first of these cases until GitHub cancelled it after six hours. Those two cases now use a non-blocking client that sends what fits and lets the connection read before sending the rest. Reproduced on Linux by shrinking the client's send buffer to 8 KiB; all nine cases pass there and on Windows.

A test that hangs held its job until GitHub cancelled it after six hours, and the run showed only "cancelled", not which test hung. ctest now stops each test after five minutes and reports it as a failure under its name. The slowest test takes about 30 seconds.

Mosch0512

The fix works: the macOS editor-ON job passes, and each local-socket case takes about 0.01 s there. The comments below cover what happens the next time something stalls, and what the tests actually check. The first two (the send loops and BufferInto no longer require progress) are the ones I'd fix here; the rest can be follow-ups.

Mosch0512 · 2026-09-29T20:11:39Z

-        const int sent = SendAll(client, batch.substr(offset, size));
-        REQUIRE(sent > 0);
+        const int sent = SendWhatFits(client, batch.substr(offset, size));
+        REQUIRE(sent >= 0);


The loop no longer requires progress. REQUIRE(sent > 0) became REQUIRE(sent >= 0), so a pass that sends nothing is accepted, and nothing limits how many such passes there are. If a platform keeps answering WOULDBLOCK while ReadAvailable() reads nothing (reading paused, delivery not yet visible, or an all-or-nothing send with a buffer under 16 KiB), this spins at 100% CPU until ctest's 300 s limit, and forever when the binary is run directly or from the IDE. The pacing loop at line 582 has the same problem.

The other waits in this file (AcceptWithin, ReadLineWithin) use a deadline. These loops could too, or fail after N passes without progress.

Fixed in d8086b7. The three loops are now one SendWhileServing, which stops once the client has not got a byte out for StallTimeout (5 s) and sleeps PollInterval between passes that sent nothing, so it no longer spins either. The pacing loop goes through the same helper.

Mosch0512 · 2026-09-29T20:11:40Z

-        const int sent = SendAll(client, payload.substr(offset, size));
-        if (sent <= 0)
+        const int sent = SendWhatFits(client, payload.substr(offset, size));
+        if (sent < 0)


BufferInto can spin forever. A send of 0 bytes now counts as success. The helper's contract is to buffer without draining lines, and that is exactly when the read pause kicks in. With at least ReadPauseBytes of complete lines, ReadAvailable() stops calling recv() and the client's buffer fills. SendWhatFits then returns 0 while ReadAvailable() keeps returning true, so offset never advances.

This can't happen yet (the only caller sends a blob with no newlines), but the helper should fail on a pass that makes no progress.

Fixed in d8086b7. BufferInto is gone: the blob goes through SendWhileServing(..., Drain::Nothing), which has the same stall limit. The pacing case in 8356c0a depends on that. With Drain::Nothing it runs into the read pause, and the stall limit (PauseSettleTimeout, 200 ms there) is what ends that phase.

Mosch0512 · 2026-09-29T20:11:40Z

+        REQUIRE(sent >= 0);
        offset += static_cast<std::size_t>(sent);

        REQUIRE(connection->ReadAvailable());


This test never reaches the read pause. Every pass drains all lines with TakeLine, so the inbox holds at most one chunk and m_inbox.size() - PendingLineBytes() >= ReadPauseBytes never fires. The macOS run finishes this case in 0.01 s. The comment at line 574 ("the buffer stays at the pause mark") doesn't hold, and a regression that removed the pause would still pass.

Now that the client is non-blocking, the test can exercise the pause: send without draining until SendWhatFits returns 0, check that the connection is open and the inbox is bounded, then drain and count.

Fixed in 8356c0a, along the lines you suggested. The case sends with Drain::Nothing until the writer has been held off for PauseSettleTimeout. It then checks that the connection is open, that the batch did not all go out, and that the buffered lines come to at least ReadPauseBytes and less than ReadPauseBytes + ReadChunkBytes. After that it serves the rest and checks that every line arrives. With the pause disabled in LocalSocket.cpp, the case fails on REQUIRE(behind.bytesSent < batch.size()). The stale comment is gone.

Mosch0512 · 2026-09-29T20:11:40Z

+int SendWhatFits(SOCKET handle, std::string_view payload)
+{
+    const int sent = SendAll(handle, payload);
+    if (sent < 0 && WSAGetLastError() == WSAEWOULDBLOCK)


This path only runs on macOS. Linux and Windows buffer more than a 16 KiB chunk, so their jobs never take the return 0 branch or a partial send. A later change that mishandles a partial send would pass there and fail only on macOS again.

Setting a small SO_SNDBUF on the non-blocking client (how you reproduced this on WSL) would make every platform take this path.

Fixed in d8086b7. ConnectForLargeWrites asks for a 4 KiB SO_SNDBUF (ClientSendBufferBytes; Linux doubles it to 8 KiB), less than one 16 KiB chunk, so Linux now takes partial sends in every large-payload case. Windows accepts the option on AF_UNIX sockets (the REQUIRE on it passes there). The paused phase of the pacing case also runs the zero-byte SendWhatFits path on every platform.

Mosch0512 · 2026-09-29T20:11:40Z

+#
+# Each test gets five minutes (ctest --timeout; the slowest takes about 30
+# seconds). A test that hangs then fails under its own name instead of
+# holding the job until GitHub cancels it after six hours.


Hangs outside ctest still run for six hours. --timeout only limits tests. doctest_discover_tests runs each test binary with --list-test-cases as a POST_BUILD step, through execute_process with no timeout (tests/third_party/doctest/doctestAddTests.cmake:38). A binary that hangs at start-up therefore stalls the Build step instead, and so would a stuck restore or configure.

A job-level timeout-minutes (above the ~35 min the Windows editor-ON job takes) would cover every step.

Fixed in 0d4e689: every Fork CI job has timeout-minutes: 90. Over the last 40 runs the slowest job (Windows, editor ON) took 35 minutes, so that is over twice the worst case. A stall in discovery, restore or configure now ends after an hour and a half instead of six.

Mosch0512 · 2026-09-29T20:11:40Z

@@ -495,8 +519,8 @@ TEST_CASE("Local socket bounds the unterminated tail, not a pipelined batch [cor
    while (offset < batch.size())
    {
        const std::size_t size = std::min(ChunkBytes, batch.size() - offset);


The same loop appears in three places. This send-then-read loop, each copy with its own ChunkBytes = 16 * 1024, is here, in the pacing test (575–592) and in BufferInto. This PR had to make the same edit in all three, and a fix like a progress deadline would have to land three times too.

One helper (e.g. ServeWhileSending(client, connection, payload, drain), returning the lines taken) with a single file-scope ChunkBytes would replace all three (CODING_RULES rule 4).

Fixed in d8086b7: SendWhileServing(client, connection, payload, drain) with one file-scope ChunkBytes. It returns a Delivery (bytes sent and lines taken) rather than only the line count, because the pacing case needs to know where the writer was held off.

Mosch0512 · 2026-09-29T20:11:40Z

+// full, until the connection reads from it; -1 when the socket failed.
+int SendWhatFits(SOCKET handle, std::string_view payload)
+{
+    const int sent = SendAll(handle, payload);


SendAll doesn't send everything. It makes a single send() call (line 118), which may be partial, and SendWhatFits exists to handle exactly that partial send. Read together, SendWhatFits calling SendAll suggests the inner call loops until everything is written. Renaming it (SendOnce) or inlining it here would fix that (CODING_RULES rule 5: name things precisely).

Renamed to SendOnce in d8086b7, with a comment saying it may take only part of the payload.

Mosch0512 · 2026-09-29T20:11:40Z

-// only serves a few requests per frame interleave.
+// only serves a few requests per frame interleave. The client must be
+// non-blocking (MakeNonBlocking).
 bool BufferInto(SOCKET client, Core::Platform::LocalSocketConnection& connection, std::string_view payload)


The non-blocking requirement is only a comment. Every caller has to remember a separate REQUIRE(MakeNonBlocking(client)) after ConnectTo. A new test that forgets it passes on Linux and Windows and hangs only on macOS, which is this PR's bug again. A ConnectNonBlocking(path) helper, or BufferInto setting the mode itself, would enforce it.

Enforced by the type in d8086b7. SendWhileServing and SendWhatFits take a LargeWriteClient, and only ConnectForLargeWrites makes one (non-blocking, small send buffer), so passing a blocking client no longer compiles.

Mosch0512 · 2026-09-29T20:11:40Z

    return ::send(handle, payload.data(), static_cast<int>(payload.size()), 0);
 }

+// A test that writes more than the socket buffer holds, from the same thread


Two comments elsewhere in the file are now stale. The file header (line 5) still says the client is "a plain blocking socket", and ConnectTo (line 91) still says "Blocking client side". Two cases now make the client non-blocking. Updating both would keep a reader from sending a large payload from the reading thread because the comments told them the client blocks.

Both updated in d8086b7. The header now calls the client "a plain socket created by the test". ConnectTo says it blocks, which only suits payloads the kernel buffers whole, and points at ConnectForLargeWrites for larger ones.

Mosch0512 · 2026-09-29T20:11:40Z

        const std::size_t size = std::min(ChunkBytes, batch.size() - offset);
-        const int sent = SendAll(client, batch.substr(offset, size));
-        REQUIRE(sent > 0);
+        const int sent = SendWhatFits(client, batch.substr(offset, size));


Each pass copies a chunk. batch is a std::string, so batch.substr(offset, size) allocates and copies up to 16 KiB on every pass, including passes where nothing is sent. On macOS each pass copies 16 KiB to send at most 8 KiB. std::string_view(batch).substr(offset, size) avoids the copy (the same applies at line 582).

Fixed in d8086b7. SendWhileServing takes a std::string_view and slices it, so no pass copies a chunk. The pacing case's second phase passes a view of the rest of the batch.

The socket test had its own copy of the transport's SetNonBlocking and a narrower WouldBlock that missed EAGAIN and EINTR, so it failed a send the transport would have retried. Both now live in NonBlockingSocket.h and the transport and the test call the same code.

The send-then-read loop was written out three times, and after the macOS fix none of the copies required progress: a pass that sent nothing was accepted, so a connection that stopped reading kept the loop spinning at full CPU until ctest stopped it, or forever outside ctest. SendWhileServing replaces all three. It gives up once the client has not got a byte out for five seconds, sends from a string_view instead of copying each chunk, and either takes the lines as they arrive or leaves them buffered. The final count waits for the end of the stream with a deadline instead of assuming the last chunk is readable the moment send() returns. Only ConnectForLargeWrites makes a client that SendWhileServing accepts. It is non-blocking, so forgetting the mode no longer compiles, and it asks for a send buffer smaller than one chunk, so Linux and Windows take the partial sends that only macOS took before. SendAll made a single send() call and is now SendOnce, and the comments that still called the client blocking are updated.

Every pass drained all lines, so the inbox never held more than a chunk and the read pause never fired: a change that removed it would still have passed. The case now sends without serving first, like a frame loop that has fallen behind, until the writer is held off. The buffered lines must reach the pause mark and stop within one read of it, with the connection still open. Serving then resumes, and the rest of the batch has to arrive in full. With the pause disabled in the transport the case fails.

ctest --timeout covered only the three Fork CI jobs: ci.yml's test runs and local runs had no limit, and the 300 was written out three times. mu_add_test now gives every discovered case a TIMEOUT from one named setting, MU_TEST_CASE_TIMEOUT_SECONDS (120 s; the slowest case takes about a second), so the limit applies wherever ctest runs. A per-test limit does not cover the rest of a job: doctest's discovery runs each test binary during the build with no timeout, and a restore or configure can stall too. Each Fork CI job now has timeout-minutes: 90, over twice the slowest job (Windows, about 35 minutes).

Mosch0512 added 2 commits September 29, 2026 21:18

Mosch0512 commented Sep 29, 2026

View reviewed changes

Mosch0512 added 4 commits September 29, 2026 22:21

Mosch0512 mentioned this pull request Sep 29, 2026

fix(ci): stop the macOS editor-ON job hanging in the local socket tests sven-n/MuMain#660

Merged

3 tasks done

Mosch0512 merged commit 4b3da1d into main Sep 29, 2026
35 checks passed

Mosch0512 deleted the fix/macos-local-socket-hang branch September 29, 2026 22:58

Conversation

Mosch0512 commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Testing

Uh oh!

Mosch0512 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Mosch0512 commented Sep 29, 2026 •

edited

Loading