Skip to content

Back-merge riina #13 — make the OpenSSL wire differential wait for its own server - #83

Merged
ib823 merged 4 commits into
mainfrom
claude/riina-backmerge-13
Aug 26, 2026
Merged

Back-merge riina #13 — make the OpenSSL wire differential wait for its own server#83
ib823 merged 4 commits into
mainfrom
claude/riina-backmerge-13

Conversation

@ib823

@ib823 ib823 commented Aug 25, 2026

Copy link
Copy Markdown
Owner

A sixth riina commit, found by re-checking the mirror before touching it. This one fixes a flake this session hit twice.

Why it matters here, not just there

openssl_wire_differential failed my pre-push gate once and my full-suite run once during this session. Both times it passed in isolation, so I called it a flake and moved on — correct as far as it went, but I never diagnosed it. riina did:

s_server has to bind its own port, so candidate_port binds an ephemeral port and drops it, leaving a window in which a sibling test can take the same number. The old code then polled TcpStream::connect and treated the first success as "the server is up". It is not:

  • two tests draw the same port;
  • one s_server binds it, the other fails with Address already in use and exits — with status 0, so the exit code says nothing;
  • the loser's connect succeeds against the winner's server;
  • that server runs with -naccept 1, has already spent its one accept on its own client, and closes;
  • the intruding connection is reset and the test dies on the next write.

The existing retry-on-timeout could never fire, because connect succeeded every time. The failure "looked nothing like the port race it was: no timeout, no did not come up, just write_all dying on a broken pipe about 0.2s in."

That is a better diagnosis than "flaky, passes in isolation", which is what I settled for.

Verification

  • The previously-flaky suite run 5× consecutively: 7 passed, 0 failed each time
  • 3323 Rust tests, 0 failed (+1)
  • clippy clean; audit-docs.sh 0 discrepancies, 0 warnings

Cherry-picked cleanly onto main, authorship preserved.

Mirror status

riina/main is now e71160270. It has moved three times during this session's work — c83f0e0c9f20e7ba8a (became #82) → dd4c6ff8a (metrics-only) → e71160270 (this).

The standing rule holds: sync-public.sh only with --reconcile, and re-fetch riina and diff its tree before any mirror push. A remembered survey is not sufficient — checking rather than remembering is what caught both #11 and this one.


Generated by Claude Code

ib823 and others added 2 commits August 25, 2026 15:45
…#13)

`openssl_wire_differential` failed roughly 1 run in 60 under parallel load,
including once in CI. The failure looked nothing like the port race it was:
no timeout, no "did not come up", just `write_all` dying on a broken pipe
about 0.2s in.

The mechanism

`s_server` has to bind its own port, so `candidate_port` binds an ephemeral
port and drops it, leaving a window in which a sibling test can take the same
number. The old code then polled `TcpStream::connect` and treated the first
success as "the server is up". It is not:

  - two tests draw the same port;
  - one `s_server` binds it, the other fails with `Address already in use`
    and exits — with status 0, so the exit code says nothing;
  - the loser's `connect` succeeds against the WINNER's server;
  - that server runs with `-naccept 1`, has already spent its one accept on
    its own client, and closes;
  - the intruding connection is reset and the test dies on the next write.

The existing retry-on-timeout could never fire, because `connect` succeeded
every time. The comment on `candidate_port` promised the caller retried "when
the child fails to come up" — but nothing ever checked whether the child that
came up was ours.

The fix

Make the child SAY it bound the port. `s_server` prints a bare `ACCEPT` line
once its bind and listen succeed, so the wait loop now polls for that line and
for the child having exited, and connects only after seeing `ACCEPT`. A child
that lost the race exits without printing it, which costs a retry on a fresh
port rather than a connection to somebody else's server. Reaching `connect` now
implies our own successful bind, structurally.

`-quiet` had to go, since it suppresses `ACCEPT`. That has a non-obvious
consequence: a non-quiet `s_server` also multiplexes its own stdin into the
session and shuts the connection down on stdin EOF, so `Stdio::null()` made it
close on every client before answering. stdin is now an open pipe that is never
written to and never closed. Nothing on the wire changes either way — `-quiet`
only ever governed the server's own stdout.

Pinned by a test, not by hope

A collision is rare enough that 240 stress runs under load produced none, so
waiting for one to prove the recovery works is not a plan. `openssl_server_on`
takes the port source as a parameter, and `an_occupied_port_costs_a_retry_not_a
_failure` hands out a port the test itself holds open. Verified to FAIL against
the old connect-poll logic and pass against the new — a recovery path that only
runs once in hundreds of CI runs is otherwise a path nobody has seen work.

Also fixed: a short-read assumption

`parses_a_real_openssl_client_hello` did a single `read` and assumed a whole
record arrived. TCP is a byte stream and a single read is entitled to return a
prefix, so under load this would have failed inside `parse_record` — reporting
a parser bug where the real fault is a short read. It now accumulates until a
complete record parses, which is what `exchange_client_hello` already did; this
side was the inconsistent one.

Verification: 0 failures in 200 stress runs (120 sequential under background
build load, 80 at 10x concurrency), against 1 in 60 before. Full workspace
suite green.
The test riina's #13 adds moves the count the doc banners quote. Re-derived by
command.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu28z8CdRQ1SLzTv8yszth
ib823 pushed a commit that referenced this pull request Aug 25, 2026
Byproducts of scripts/deploy-website.sh — metrics provenance stamp, rebuilt
playground WASM, report timestamps. Derived counts unchanged.

riina main was again NOT pushed: it carries commit e71160270 (the OpenSSL wire
differential fix), which is back-merging via PR #83 and does not yet exist here.
This chain used --reconcile throughout; only gh-pages was updated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu28z8CdRQ1SLzTv8yszth
claude added 2 commits August 25, 2026 17:15
main moved during the #82 sync chain — the 3318 -> 3322 banner sync plus deploy
artifacts. 120 files conflicted, ALL generated content: metrics.json and the doc
banners quoting it (3322 on main vs 3323 here). No source conflicts; the
OpenSSL port-race fix is untouched.

Resolved to main's side and then RE-DERIVED rather than hand-picked, per Prime
Directive 8. Re-verified on the merged tree: 3323 tests / 0 failed, 47 banners
re-synced, audit-docs 0 discrepancies and 0 warnings.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu28z8CdRQ1SLzTv8yszth
riinac verify --full on the merged tree: 3323 Rust tests, 0 clippy warnings,
0 admits, 0 axioms.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu28z8CdRQ1SLzTv8yszth
@ib823
ib823 marked this pull request as ready for review August 26, 2026 22:31
@ib823
ib823 merged commit f87d6ae into main Aug 26, 2026
11 checks passed
ib823 pushed a commit that referenced this pull request Aug 26, 2026
Regenerated on main after merging the riina #13 back-merge. Counts unchanged at
3323 tests / 172 examples; only provenance moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu28z8CdRQ1SLzTv8yszth
ib823 pushed a commit that referenced this pull request Aug 26, 2026
Byproducts of scripts/deploy-website.sh — metrics provenance stamp, rebuilt
playground WASM, report timestamps. Derived counts unchanged.

riina main was again NOT pushed. It moved twice more during this chain, to
21854c54e (metrics-only) and then 3fc31f2e9, which adds a MIRROR-LOCAL
scripts/mirror-propagate-metrics.sh. That one is deliberately not back-merged:
its own commit message says it exists because the mirror lacks
scripts/sync-metrics.sh, and that it is named distinctly so the real script can
land later without a collision. proof already has the real one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uu28z8CdRQ1SLzTv8yszth
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants