[monotouch-test] Add timeouts to all Thread.Join calls#25644
Conversation
Replace all Thread.Join() calls (no timeout) with Thread.Join(TimeSpan) + Assert.That to fail the test if the thread doesn't complete within a reasonable time. Timeouts are chosen based on the work each thread does: - 5s for simple/fast operations (object creation, property checks) - 10s for I/O or RunLoop-based threads (timers, network listeners) - 30s for heavy workloads (10k iterations, GPU image processing) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR updates the tests/monotouch-test suite to avoid indefinite hangs by replacing Thread.Join() (no timeout) with Thread.Join(TimeSpan) and an Assert.That to fail tests when worker threads don’t complete within a bounded time.
Changes:
- Replaced unbounded
Thread.Join()calls with boundedJoin(TimeSpan)+Assert.Thatacross multiple tests. - Selected per-test timeouts (5s/10s/30s) based on expected workload characteristics.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/monotouch-test/SystemConfiguration/NetworkReachabilityTest.cs | Adds 5s Join timeouts for background-thread reachability GC/notification tests. |
| tests/monotouch-test/ObjCRuntime/RuntimeTest.cs | Adds 5s Join timeouts for runtime/GC/thread interaction tests. |
| tests/monotouch-test/ObjCRuntime/RegistrarTest.cs | Adds a 30s Join timeout for a heavy block-collection workload. |
| tests/monotouch-test/Foundation/TimerTest.cs | Adds 10s Join timeouts for runloop/timer worker threads. |
| tests/monotouch-test/Foundation/ThreadTest.cs | Adds a 5s Join timeout for a simple thread completion check. |
| tests/monotouch-test/Foundation/NSStreamTest.cs | Adds a 10s Join timeout to the listener thread used for stream socket tests. |
| tests/monotouch-test/Foundation/NotificationCenter.cs | Adds per-thread Join timeouts to the multithreaded add/remove observer stress test. |
| tests/monotouch-test/CoreImage/CIKernelTests.cs | Adds a 30s Join timeout for CoreImage processing on a separate thread. |
| tests/monotouch-test/CoreAnimation/LayerTest.cs | Adds 5s/10s Join timeouts for CALayer-related thread scenarios. |
| Assert.That (result [i], Is.EqualTo (send [i] * 10)); | ||
| listenThread.Join (); | ||
| Assert.That (listenThread.Join (TimeSpan.FromSeconds (10)), Is.True, "listenThread.Join timed out"); | ||
| listener.Stop (); | ||
| read.Close (); | ||
| write.Close (); |
| thread.Start (); | ||
| thread.Join (); | ||
| Assert.That (thread.Join (TimeSpan.FromSeconds (5)), Is.True, "Thread.Join timed out"); |
| thread.Start (); | ||
| thread.Join (); | ||
| Assert.That (thread.Join (TimeSpan.FromSeconds (5)), Is.True, "Thread.Join timed out"); |
| t1.Start (); | ||
| t1.Join (); | ||
| Assert.That (t1.Join (TimeSpan.FromSeconds (5)), Is.True, "Thread.Join timed out"); |
| thread.Start (); | ||
| thread.Join (); | ||
| Assert.That (thread.Join (TimeSpan.FromSeconds (30)), Is.True, "Thread.Join timed out"); |
| t.Start (); | ||
| t.Join (); | ||
| Assert.That (t.Join (TimeSpan.FromSeconds (30)), Is.True, "Thread.Join timed out"); |
| thread.Start (); | ||
| thread.Join (); | ||
| Assert.That (thread.Join (TimeSpan.FromSeconds (10)), Is.True, "Thread.Join timed out"); |
This comment has been minimized.
This comment has been minimized.
If a Join times out and the thread is a foreground thread, it can keep the test process alive. Mark all affected threads as background so they don't block process exit on timeout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
✅ [PR Build #9fd53a3] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #9fd53a3] 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 #9fd53a3] 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 #9fd53a3] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 193 tests passed 🎉 Tests counts✅ 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 |
Replace all
Thread.Join()calls (no timeout) in monotouch-test withThread.Join(TimeSpan)+Assert.Thatto fail the test if the thread does not complete within a reasonable time, instead of hanging indefinitely.Additionally, all affected threads are marked as background threads (
IsBackground = true) so that if a Join times out, the orphaned thread does not keep the test process alive.Timeouts are chosen based on the work each thread does:
🤖 Pull request created by Copilot