fix(ci): stop the macOS editor-ON job hanging in the local socket tests - #660
Merged
Merged
Conversation
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.
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).
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.
Summary
On macOS with the control socket enabled,
ctesthangs inLocal socket bounds the unterminated tail, not a pipelined batch [core][local-socket]until CI 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), sosend()waited for a read that could never start. Linux and Windows buffer more than one chunk, so they passed.ci.ymlbuilds macOS without the editor, so these tests are not compiled there.The transport's behaviour is unchanged; only the test client was wrong. Reviewed on the fork in Mosch0512#92.
The hang
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.ConnectForLargeWritesmakes a clientSendWhileServingaccepts: 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.send()returns.Test coverage
Shared code
Core/Platform/NonBlockingSocket.hholdsEnableandWouldBlock, used by bothLocalSocket.cppand the test. The test previously had its own copies, and itsWouldBlockmissedEAGAINandEINTR.Time limits
mu_add_testgives every discovered test case aTIMEOUTofMU_TEST_CASE_TIMEOUT_SECONDS(120 s; the slowest case takes about a second), so a hanging case fails under its own name wherever ctest runs.fork-ci.ymljob hastimeout-minutes: 90(the slowest, Windows, takes about 35 minutes). That also covers doctest's test discovery during the build, restore and configure.Testing
SO_SNDBUFto macOS's 8 KiB. All 9 local-socket cases pass there, with the default buffer and on Windows (MSVC).TIMEOUTproperty is stopped and reported by name.Related issues
Closes #633
Checklist
docs/CODING_RULES.md.docs/build/README.md). The local-socket tests locally on Linux and Windows; the full project in Fork CI on Windows, Linux and macOS.