Tests: stop handing core ports that were just released - #192
Merged
Merged
Conversation
CI failed once in tw-control's remote tests with `gw.listen.port_taken` on 127.0.0.1:34393 (narrowing_allow_from_closes_the_connections_it_no_longer_lets_in). free_port() bound 127.0.0.1:0, read the port and closed the socket, and the test then wrote that number into the config for core to bind. A released port goes back to the system's ephemeral range, which parallel tests draw from whenever they bind port 0 or open a connection. Linux picks those ports at random, so now and then another test took the number before core bound it. The same "bind 0, read the port, release it" step was in 38 places that start a gateway with tw_gateway::serve, and in the hot-reload, listen and remote-port tests. - tw_gateway::serve binds before it returns and hands back the address it bound. The 38 call sites pass 127.0.0.1:0 and use the returned address, so the system picks the port and nobody can take it in between. serve is only used by tests. - Where the number has to exist before anything binds it, tests take it from spare_port(): the remote control port (the config refuses 0 on purpose, config.remote_port_zero), the port PUT /listen saves, and the hot-reload tests that move between two ports or keep one port across addresses. spare_port() picks at random from REMOTE_PORT_RANGE (20000-32000), below the ephemeral ranges of Linux, macOS and Windows, so the system never hands these ports out itself. It holds each number with a UDP socket for the life of the process. UDP and TCP ports are separate, so core still binds the TCP port, but another test, or another test process running at the same time, cannot get the same number. - The mirror case: a port that should have nobody listening (an unreachable upstream or proxy) was also a released ephemeral port, which a parallel test could start listening on. Those come from the same range now, held the same way. Reproduced on macOS, where ephemeral ports are handed out in sequence and the race does not show without help: a background process walks the ephemeral range with bind(127.0.0.1:0) while holding the last 4000 sockets. Running the 20 affected test binaries at once, six rounds, origin/main failed 12 and 13 of 120 runs (remote, listen, hotreload; the remote failures are the same port_taken as on CI). With this change, 0 of 120 in each of three such runs. Sixteen copies of the remote test binary at once, 20 rounds: 0 of 320. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fylorn
force-pushed
the
fix/tests-let-core-pick-its-port
branch
from
September 25, 2026 01:47
d43563b to
63904a0
Compare
Merged
fylorn
added a commit
that referenced
this pull request
Sep 25, 2026
v0.48.0 was published before the template landed, so its page holds only GitHub's generated list. release-notes/0.48.0.md summarizes it from #191-#198 so the page can be rewritten from the template: - upgrade notes: CONTROL_API_VERSION 21, so ThinkWatch Lite 2026.9.16 (core 0.47.0, protocol 20) does not connect to it and a server used with that app stays on 0.47.0; the request store's schema 20, which empties the request history on the first start (and again on the way back); the /in-flight shape, RequestStarted.session and the removed ChatgptUsage fields; - session and route on the start event, requests a rule decided without an upstream, the replayable /in-flight snapshot and the new /live fields, and /summary/routes (#197); - per-group unpriced and no-usage counts, and security log totals (#193); - the signed-in account on a ChatGPT account upstream (#195); - releases published from one job, the server guide and the crate metadata (#191), and the test port fix (#192). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fylorn
added a commit
that referenced
this pull request
Sep 25, 2026
v0.48.0 was published before the template landed, so its page holds only GitHub's generated list. release-notes/0.48.0.md summarizes it from #191-#198 so the page can be rewritten from the template: - upgrade notes: CONTROL_API_VERSION 21, so ThinkWatch Lite 2026.9.16 (core 0.47.0, protocol 20) does not connect to it and a server used with that app stays on 0.47.0; the request store's schema 20, which empties the request history on the first start (and again on the way back); the /in-flight shape, RequestStarted.session and the removed ChatgptUsage fields; - session and route on the start event, requests a rule decided without an upstream, the replayable /in-flight snapshot and the new /live fields, and /summary/routes (#197); - per-group unpriced and no-usage counts, and security log totals (#193); - the signed-in account on a ChatGPT account upstream (#195); - releases published from one job, the server guide and the crate metadata (#191), and the test port fix (#192). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fylorn
added a commit
that referenced
this pull request
Sep 25, 2026
…summary (#196) * ci(release): write the release page from a template, with an English summary #191 made the publish job write the release once, with GitHub's generated list of pull requests as its only text and the tag as its title. A release is now titled "ThinkWatch Core <version>", and its text has, in order: - an English summary from release-notes/<version>.md, when that file exists; - a table of the files for each platform; - the commands that install this version on a Linux server and switch an existing installation to it (install.sh --version, twcore upgrade --version --restart); - how to verify a download against its .sha256; - GitHub's generated list of pull requests. scripts/release_notes.py builds the text; the publish job fetches the generated list itself (releases/generate-notes) and hands the finished text to action-gh-release, which no longer generates anything. The "only once" rule from #191 stays: a release that already exists keeps its text. A rehearsal (workflow_dispatch) now writes the text into the run summary, using the version in Cargo.toml. scripts/release_notes_test.py checks that the table links exactly the files in release.yml's FILES list, that the install and upgrade options exist, and that every file in release-notes/ renders; the Linux CI job runs it before compiling. release-notes/0.47.0.md summarizes 0.47.0 from #183-#190; the live v0.47.0 release page now carries it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * release-notes: summarize 0.48.0 v0.48.0 was published before the template landed, so its page holds only GitHub's generated list. release-notes/0.48.0.md summarizes it from #191-#198 so the page can be rewritten from the template: - upgrade notes: CONTROL_API_VERSION 21, so ThinkWatch Lite 2026.9.16 (core 0.47.0, protocol 20) does not connect to it and a server used with that app stays on 0.47.0; the request store's schema 20, which empties the request history on the first start (and again on the way back); the /in-flight shape, RequestStarted.session and the removed ChatgptUsage fields; - session and route on the start event, requests a rule decided without an upstream, the replayable /in-flight snapshot and the new /live fields, and /summary/routes (#197); - per-group unpriced and no-usage counts, and security log totals (#193); - the signed-in account on a ChatGPT account upstream (#195); - releases published from one job, the server guide and the crate metadata (#191), and the test port fix (#192). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * release-notes(0.48.0): say where the key is kept without naming the keychain Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
The failure
CI failed once on Linux in tw-control's remote-port tests (job, first attempt on #191):
free_port()bound127.0.0.1:0, read the port and closed the socket. The test then wrote that number into the config for core to bind. A released port goes back to the system's ephemeral range, and parallel tests draw from that range whenever they bind port 0 or open a connection. Linux picks those ports at random, so now and then another test took the number before core bound it.The same "bind 0, read the port, release it" step was in 38 places that start a gateway with
tw_gateway::serve, and in the hot-reload,PUT /listenand remote-port tests.The change
tw_gateway::servebinds before it returns and hands back the address it bound. The 38 call sites pass127.0.0.1:0and use the returned address, so the system picks the port and nothing can take it in between.serveis only used by tests (twcore serve --portgoes throughserve_at), and neither the desktop app nor ThinkWatch Enterprise depends on tw-gateway.Where the number has to exist before anything binds it, tests take it from
spare_port():config.remote_port_zero), so core cannot pick it the way--port 0lets it pick the gateway port;PUT /listensaves (it refuses 0 too);alland a single address.spare_port()picks at random fromREMOTE_PORT_RANGE(20000–32000), below the ephemeral ranges of Linux (32768–60999), macOS and Windows (49152–65535), so the system never hands these ports out itself. It holds each number with a UDP socket for the life of the test process. UDP and TCP ports are separate, so core still binds the TCP port, but another test, or another test process running at the same time (two worktrees running the suite), cannot get the same number. An earlier version that only kept a list inside the process collided about once in 120 runs with 12 copies of the remote tests at once. With the UDP hold, 16 copies over 20 rounds had no failures (0 of 320).Ports that should have nobody listening (an unreachable upstream or proxy in
passthrough,endings,ws,live_stateand thel1unit tests) were also released ephemeral ports, which a parallel test could start listening on. They come from the same range now, held the same way. (A socket that is bound but not listening does not work for this: macOS drops the SYN instead of refusing it, and the connection times out.)No assertion changed. The remote-port tests still check the exact port in the status, the hot-reload tests still move between two concrete ports, and the old port is still expected to refuse connections after a move.
Reproducing it
On macOS the race does not show by itself: ephemeral TCP ports are handed out in sequence (
net.inet.tcp.randomize_ports: 0), so a released port is not reused until the counter wraps. To stand in for Linux, a background process walks the ephemeral range withbind(127.0.0.1:0)and holds the last 4000 sockets. All 20 affected test binaries then run at the same time, once per round:main(8095405)main(8095405)The
mainfailures in the remote tests are the samegw.listen.port_takenas on CI. In listen and hotreload they are the gateway failing to bind its configured port.Checks
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,cargo clippy --workspace --lib -- -D warnings, andcargo test --workspacewith the proxy variables unset: 1581 passed, 0 failed, 4 ignored.🤖 Generated with Claude Code