Repository navigation
Conversation
sws updates its connection-monitor state only when it parses a request. Since 5232c16 it no longer clears the state after the first logged disconnect, so the state a test sets with connection-monitor remains after that test ends. If the next test that uses the same server closes a connection before sws has parsed any request of that test, sws logs [DISCONNECT] even if that test does not use connection-monitor. Tests 3231 and 3232 do that: they close a CONNECT_ONLY connection, which never sent anything, before the HTTP transfer sends its request. They fail with an unexpected [DISCONNECT] in server.input whenever the state is still enabled from a previous test, such as 3223 to 3226. With parallel test runners this depends on which runner picks up the test, so the failures look random. Most Debian package builds of 8.23.0-rc1 failed like this. To reproduce, run this in the tests directory: ./runtests.pl 3226 3231 When a connection closes while the test number is unset (DOCNUMBER_NOTHING), read it from server.cmd, as sws already does for requests without one. Then use the connection-monitor setting of that test, or disable connection monitoring if there is no test number. A test using connection-monitor now gets [DISCONNECT] logged for a connection closed before its first request, regardless of the previous test. Ref: curl#22902 Follow-up to 5232c16 curl#22159
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The reviewed change resolves the stale monitoring state with no unresolved issues.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes stale connection-monitor state when sws connections close before sending a request.
Changes:
- Reads the active test number from
server.cmdon early disconnects. - Reloads that test’s server commands or disables monitoring when unavailable.
| File | Description |
|---|---|
tests/server/sws.c |
Refreshes connection-monitor state for pre-request disconnects. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bagder
approved these changes
Sep 24, 2026
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.
Note: I've used AI to debug this and get to a patch, I've confirmed through testing that this fixes the issue but I'm not familiar enough with the sws test server to be 100% confident on the fix, in the worst case this can still serve as a bug report.
sws updates its connection-monitor state only when it parses a request. Since 5232c16 it no longer clears the state after the first logged disconnect, so the state a test sets with connection-monitor remains after that test ends. If the next test that uses the same server closes a connection before sws has parsed any request of that test, sws logs [DISCONNECT] even if that test does not use connection-monitor.
Tests 3231 and 3232 do that: they close a CONNECT_ONLY connection, which never sent anything, before the HTTP transfer sends its request. They fail with an unexpected [DISCONNECT] in server.input whenever the state is still enabled from a previous test, such as 3223 to 3226. With parallel test runners this depends on which runner picks up the test, so the failures look random. Most Debian package builds of 8.23.0-rc1 failed like this. To reproduce, run this in the tests directory:
./runtests.pl 3226 3231
When a connection closes while the test number is unset (DOCNUMBER_NOTHING), read it from server.cmd, as sws already does for requests without one. Then use the connection-monitor setting of that test, or disable connection monitoring if there is no test number. A test using connection-monitor now gets [DISCONNECT] logged for a connection closed before its first request, regardless of the previous test.
Ref: #22902
Follow-up to 5232c16 #22159
Debian build/test logs at
https://buildd.debian.org/status/package.php?p=curl
https://buildd.debian.org/status/fetch.php?pkg=curl&arch=amd64&ver=8.23.0%7Erc1-1&stamp=1790130729&raw=0