Bound remaining hangable operations in test scripts#10900
Open
julek-wolfssl wants to merge 3 commits into
Open
Bound remaining hangable operations in test scripts#10900julek-wolfssl wants to merge 3 commits into
julek-wolfssl wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens scripts/*.test against indefinite hangs in CI by bounding long-running client/server/probe operations and tightening cleanup behavior, so a wedged subprocess doesn’t consume the job’s full timeout-minutes without producing logs.
Changes:
- Wrap previously-unbounded foreground clients/servers and OpenSSL invocations with
timeout -s KILL 2m, adding a macOS fallbacktimeout()shim where needed. - Bound “find a free port” scan loops (add
nc -w 1, plus a 100-port scan cap) and add-w 1to remainingncprobes/markers. - Improve cleanup to avoid hanging EXIT traps (notably for OCSP responder interop) and adjust server termination signals where timeout-wrapping is used.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/trusted_peer.test | Add timeout shim + wrap example server/client invocations; adjust cleanup signal to terminate the timeout wrapper cleanly. |
| scripts/tls13.test | Wrap TLS 1.3 example server/client/probes with timeout; adjust cleanup signal to terminate wrapped server. |
| scripts/rsapss.test | Add timeout shim; bound wolfSSL server and OpenSSL s_client runs. |
| scripts/psk.test | Add timeout shim; bound client runs (including version probe). |
| scripts/openssl.test | Add timeout shim; bound wolfSSL client and OpenSSL s_client/capability probes. |
| scripts/openssl_srtp.test | Add timeout shim; bound DTLS/SRTP wolfSSL client. |
| scripts/ocsp.test | Add timeout shim; bound external OCSP client connections. |
| scripts/ocsp-stapling2.test | Cap free-port scanning and bound multiple shutdown clients with timeout. |
| scripts/ocsp-stapling.test | Cap free-port scanning, add nc -w 1 readiness probe, and bound client invocations (incl. retry path). |
| scripts/ocsp-stapling-with-wolfssl-responder.test | Cap free-port scanning and bound client shutdown/tests with timeout. |
| scripts/ocsp-stapling-with-ca-as-responder.test | Add timeout shim; cap free-port scanning; bound client shutdown/tests with timeout. |
| scripts/ocsp-stapling_tls13multi.test | Cap free-port scanning and bound client shutdown/tests with timeout. |
| scripts/ocsp-responder-openssl-interop.test | Add timeout shim; cap free-port scanning; bound OpenSSL ocsp queries; harden responder cleanup. |
| scripts/google.test | Add timeout shim; bound external client connection. |
| scripts/external.test | Add timeout shim; bound external client connection. |
| scripts/dtlscid.test | Bound DTLS CID client invocation with timeout. |
| scripts/dtls.test | Add timeout shim; add nc -u -w 1 markers to avoid blocking probes. |
| scripts/benchmark.test | Add timeout shim; bound benchmark client invocations (servers intentionally left unwrapped). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
retest this please |
A test script that blocks forever burns the CI job's full timeout-minutes with no logs. e82ecdf and 5c5cbd3 bounded the waited-on servers; this covers the remaining hang classes in scripts/*.test: - Wrap foreground example client/server, openssl s_client, and openssl ocsp invocations in "timeout -s KILL 2m". A client wedged before or without a live peer (e.g. blocked in first-seed entropy gathering, or DTLS with no reset from a dead peer) is not bounded by its peer's timeout. - Add the macOS timeout() fallback shim to scripts that now use timeout. - Bound the get_first_free_port scan loops (nc -w 1, 100-port cap). - Add -w 1 to the remaining nc probes and dtls.test UDP pcap markers. - ocsp-responder-openssl-interop.test: bound the responder reap in cleanup: give each responder 5 s to exit after SIGTERM, then SIGKILL before waiting, so a wedged responder cannot hang the EXIT trap. - benchmark.test: bound the clients but leave the -i servers unwrapped: the script ends them with kill -6, which timeout(1) does not forward, so wrapping would orphan the server. - trusted_peer/tls13: kill the server with SIGTERM instead of SIGKILL in cleanup so the signal forwards through the timeout wrapper to the wrapped server.
- tls13.test: take the client's exit status from PIPESTATUS[0]; $? after the pipe was tee's status, hiding a client failure or 2m-timeout kill. - ocsp-responder-openssl-interop.test: only escalate to SIGKILL if the responder is still running, and make the reap counter local.
The 'Bad SSL version' probes branch on grep's status, so a client killed by the 2m timeout read as 'TLS v1.2 supported' and ran the wrong branch. Take both statuses from PIPESTATUS and fail loudly on a probe timeout.
julek-wolfssl
force-pushed
the
julek-dev/test-scripts-hang-fixes
branch
from
c7714df to
08549c6
Compare
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.
Summary
A test script that blocks forever burns the CI job's full timeout-minutes with no logs. e82ecdf and 5c5cbd3 bounded the waited-on servers; this covers the remaining hang classes in
scripts/*.test:openssl s_client, andopenssl ocspinvocations intimeout -s KILL 2m. A client wedged before or without a live peer (e.g. blocked in first-seed entropy gathering, or DTLS with no reset from a dead peer) is not bounded by its peer's timeout.timeout()fallback shim to scripts that now usetimeout.get_first_free_portscan loops (nc -w 1, 100-port cap).-w 1to the remainingncprobes anddtls.testUDP pcap markers.ocsp-responder-openssl-interop.test: bound the responder reap in cleanup: give each responder 5 s to exit after SIGTERM, then SIGKILL before waiting, so a wedged responder cannot hang the EXIT trap.benchmark.test: bound the clients but leave the-iservers unwrapped: the script ends them withkill -6, whichtimeout(1)does not forward, so wrapping would orphan the server.trusted_peer/tls13: kill the server with SIGTERM instead of SIGKILL in cleanup so the signal forwards through the timeout wrapper to the wrapped server.Test plan
scripts/*.testpass locally