Repository navigation
[tests] Await async cancellation in SecProtocolMetadataTest to avoid a hang - #25682
Conversation
…a hang SecProtocolMetadataTest.TlsDefaults starts an NWConnection on a DispatchQueue created with a 'using' block and registers a state-change handler that disposes the connection when it reaches the Cancelled/Failed state. The handler runs on that queue, but the test disposed the queue (leaving the 'using' scope) without waiting for the asynchronous cancellation to complete, and skipped the trailing Cancel () entirely whenever Assert.Fail / IgnoreInCI threw. The cancellation callback could therefore still be pending - and run on a disposed queue - long after the test finished, contributing to a process-wide hang on the macOS test run (observed as a ~20 minute timeout / "Test run crashed"). Wrap the body in try/finally and signal a ManualResetEvent from the Cancelled/Failed branch of the handler. The finally cancels the connection and waits (bounded to 10s) for that event before the queue is disposed, on every path (success, the Assert.Fail, an assertion failure inside the inner 'using', and the IgnoreInCI case). Cancel () is guarded against ObjectDisposedException so that, if the connection already failed on its own and the handler disposed it, the original test failure is preserved instead of being masked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves reliability of SecProtocolMetadataTest.TlsDefaults by ensuring NWConnection cancellation completes before the DispatchQueue used for callbacks is disposed, preventing post-test callbacks from running on a disposed queue (and avoiding potential hangs in macOS test runs).
Changes:
- Add a
ManualResetEvent(done) that is signaled when the connection reaches a terminal state (Cancelled/Failed). - Wrap test execution in
try/finallyto always cancel the connection and wait (bounded) for asynchronous cancellation before leaving the queue’susingscope. - Guard
connection.Cancel()againstObjectDisposedExceptionto preserve the original test failure/ignore outcome.
| // If we're in CI, and didn't get _any_ callbacks, then ignore the failure, since it's likely a network hiccup. | ||
| if (!anyStateChange.WaitOne (0)) | ||
| TestRuntime.IgnoreInCI ("Transient network failure - ignore in CI"); | ||
| Assert.Fail ("Connection is ready"); |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
✅ [PR Build #d519280] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #d519280] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [PR Build #d519280] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #d519280] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 1 tests failed, 192 tests passed. Failures❌ introspection tests [attempt 3]1 tests failed, 5 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
|
unrelated test failure |
SecProtocolMetadataTest.TlsDefaults starts an NWConnection on a DispatchQueue created with a 'using' block and registers a state-change handler that disposes the connection when it reaches the Cancelled/Failed state. The handler runs on that queue, but the test disposed the queue (leaving the 'using' scope) without waiting for the asynchronous cancellation to complete, and skipped the trailing Cancel () entirely whenever Assert.Fail / IgnoreInCI threw. The cancellation callback could therefore still be pending - and run on a disposed queue - long after the test finished, contributing to a process-wide hang on the macOS test run (observed as a ~20 minute timeout / "Test run crashed").
Wrap the body in try/finally and signal a ManualResetEvent from the Cancelled/Failed branch of the handler. The finally cancels the connection and waits (bounded to 10s) for that event before the queue is disposed, on every path (success, the Assert.Fail, an assertion failure inside the inner 'using', and the IgnoreInCI case). Cancel () is guarded against ObjectDisposedException so that, if the connection already failed on its own and the handler disposed it, the original test failure is preserved instead of being masked.