Repository navigation
net: end SocketServer's accept flow on a listener that can no longer accept - #881
Merged
Merged
Conversation
…accept The accept flow returned only on Cancelled; every other failed accept waited 50 ms and tried again. A listener that has stopped listening (EINVAL), a closed or foreign descriptor (EBADF/ENOTSOCK) or a non-stream socket (EOPNOTSUPP) fails every attempt the same way, so the flow woke every 50 ms forever and port() kept reporting a port nothing accepted on. Classify those as a dead listener and end the flow: drop the listener so port() reads 0 and a later listen() binds afresh, leaving connections already accepted untouched. Recognise both core-cpp spellings: BadHandle/ Unsupported, and SystemError carrying the errno, which is what the pinned core-cpp 0.5.0 reports for every failed accept. Exhaustion (EMFILE, ENFILE, ENOBUFS, ENOMEM) is still retried. The new test shuts the listening socket down underneath a running server. Without the fix it fails (port() never reads 0 within 5 s); with only the BadHandle check and no errno fallback it also fails at the current pin. Fixes #872 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WeDnphcJ7EZBLwVfE6pqZu
The clang-coverage gate dropped include/morph/net's branch coverage below its floor: the classifier's BadHandle, Unsupported, EBADF, ENOTSOCK and EOPNOTSUPP arms were never taken, because the integration test can only produce EINVAL. Make the classifier a free function, morph::net::detail:: acceptListenerIsDead, and table-test it: both spellings of every dead listener, exhaustion (retried), and per-peer codes (retried even with an errno that would mean dead under SystemError). listenerLost() loses its null check, whose false arm could not be taken: only the accept flow calls it, while its generation is current, and close() moves the generation before it resets the listener. Measured with clang 20 coverage over morph_net_tests: every arm of the classifier and of the flow's new branch taken both ways. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WeDnphcJ7EZBLwVfE6pqZu
Two assertions ran past the column limit, and clang-format wraps such a line differently across versions. A local lambda keeps every line short. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WeDnphcJ7EZBLwVfE6pqZu
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Fixes #872.
What was wrong
SocketServer::Core::acceptFlowreturned only onCancelled. Every other failedaccept()waited 50 ms and tried again. Some listeners fail every attempt in the same way:EINVAL)EBADF/ENOTSOCK)EOPNOTSUPP)For those, the flow woke every 50 ms forever, and
port()kept reporting a port that nothing accepted on.Reproduced at the current pin. #872 described this as only inferred, arriving with core-cpp ≥ 0.5.1's
BadHandle. It already happens with core-cpp 0.5.0, the version pinned today. That version reports every failed accept asSystemErrorcarrying theerrno, and the loop retried that too.The fix
morph::net::detail::acceptListenerIsDead()covers the failures that recur on every attempt. It accepts both core-cpp spellings:BadHandle/Unsupported, andSystemErrorwithEINVAL/EBADF/ENOTSOCK/EOPNOTSUPP. The supported range (MORPH_CORE_CPP_VERSION 0.5) includes 0.5.0, so both are needed.listenerLost()drops the listener.port()then reads0, which is what the docs already promise when not listening, and a laterlisten()binds again. Connections already accepted are untouched.EMFILE,ENFILE,ENOBUFS,ENOMEM) is still retried. The existing "survives accept() running out of fds" test still passes.docs/spec/core/backend.mdnow documents which accept failures are retried and which end the flow.Verification
Measured locally with clang 20.1.2, Debug, the full strict warning set, and core-cpp 0.5.0 (the pin):
shutdown()on the server's listening descriptor underneath it, then checks three things:port()reaches0listen()works againacceptListenerIsDead. It checks each condition three ways:errnothat would mean dead underSystemErrormaster): the integration test fails, becauseport()never reads0within 5 s.BadHandle/Unsupportedcheck with no errno fallback: the integration test also fails. So at the current pin, the fallback is the part that does the work.morph_net_tests: every arm of the classifier, and of the flow's new branch, is taken both ways. The first push missedinclude/morph/net's 88% floor on untested arms; that is what the table test and the second commit fix.morph_net_testssuite passes:All tests passed (1223 assertions in 216 test cases).Not verified: macOS/BSD. The integration test relies on Linux's
EINVALfromaccept()on a shut-down listener;morph::netis POSIX-only.🤖 Generated with Claude Code
https://claude.ai/code/session_01WeDnphcJ7EZBLwVfE6pqZu