socat CI: prevent rare shard hang in inverted OPENSSL tests#10931
Draft
julek-wolfssl wants to merge 3 commits into
Draft
socat CI: prevent rare shard hang in inverted OPENSSL tests#10931julek-wolfssl wants to merge 3 commits into
julek-wolfssl wants to merge 3 commits into
Conversation
The inverted testserversec OPENSSL tests (OPENSSLCERTCLIENT, the commonname test and the fips test) run a foreground one-shot SSL listener with no port-wait; their background connector exits after 2s (retry=2, 1s apart). On a loaded runner the listener can bind after the connector is already gone and then blocks in accept() forever, stalling the shard until the job-level 15-minute timeout kills the run with no diagnosable output. Seen once on a run whose re-run passed; reproduced deterministically by delaying the listener past the retry budget. - Widen the connector budget to retry=30. Retries only happen while the listener is not yet up and testserversec kills the connector as soon as the client returns, so passing runs are unaffected. - Bound each shard with timeout(1) at 10 minutes (normal shard time is ~2 min) so any future hang fails fast and the shard output names the test that was running.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the socat GitHub Actions workflow against a rare hang in inverted OPENSSL testserversec tests by widening the connector retry window and bounding each shard runtime so failures are diagnosable and don’t rely on the job-level timeout.
Changes:
- Increase socat test connector retry budget by patching
retry=2toretry=30intest.shduring CI execution. - Wrap each socat shard execution with a 10-minute
timeoutso hangs fail fast with shard-level context. - Add inline workflow comments documenting the race/hang root cause and the mitigation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [{ | ||
| "name": "socat", "build": false, "netns": true, "shards": __SHARDS__, | ||
| "run": [["bash", "-c", "set -e; ip link set lo up || true; sysctl -wq net.ipv6.conf.lo.disable_ipv6=0 || true; ip addr add ::1/128 dev lo || true; ip addr add fc00::1/128 dev lo || true; ip addr add 192.0.2.1/32 dev lo || true; sysctl -wq net.ipv6.bindv6only=0 || true; cp -a \"$SOCAT_SRC/.\" .; tests=$(seq \"$SHARD\" \"$SHARDS\" 999); SOCAT=\"$PWD/socat\" SHELL=/bin/bash ./test.sh -t 1.0 --expect-fail \"$EXPECT_FAIL\" ${tests:-0}"]] | ||
| "run": [["bash", "-c", "set -e; ip link set lo up || true; sysctl -wq net.ipv6.conf.lo.disable_ipv6=0 || true; ip addr add ::1/128 dev lo || true; ip addr add fc00::1/128 dev lo || true; ip addr add 192.0.2.1/32 dev lo || true; sysctl -wq net.ipv6.bindv6only=0 || true; cp -a \"$SOCAT_SRC/.\" .; sed -i 's/fork,retry=2,/fork,retry=30,/' test.sh; tests=$(seq \"$SHARD\" \"$SHARDS\" 999); SOCAT=\"$PWD/socat\" SHELL=/bin/bash timeout -k 10 600 ./test.sh -t 1.0 --expect-fail \"$EXPECT_FAIL\" ${tests:-0}"]] |
Fail the shard loudly if socat's test.sh no longer contains the fork,retry=2 pattern, instead of letting sed silently no-op and reintroduce the hang while the run still looks green.
Member
Author
|
retest this please |
1 similar comment
Member
Author
|
retest this please |
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 a rare socat shard hang that stalls the job until the 15-minute timeout kills it with no diagnosable output. Seen once on a run whose re-run passed; reproduced deterministically by delaying the listener past the retry budget.
Root cause
The inverted
testserversecOPENSSL tests (OPENSSLCERTCLIENT, the commonname test and the fips test) run a foreground one-shot SSL listener with no port-wait, while their background connector exits after ~2s (retry=2, 1s apart). On a loaded runner the listener can bind after the connector is already gone, then block inaccept()forever. Nothing bounds the shard, so it stalls until the job-level timeout fires and the log says nothing about which test was running.Fix
retry=30. Retries only happen while the listener is not yet up, andtestserverseckills the connector as soon as the client returns, so passing runs are unaffected — this only buys the listener a wider window to win the race.timeout(1)at 10 minutes (normal shard time is ~2 min). Any future hang then fails fast, and the shard output names the test that was running instead of dying to the opaque job-level kill.