Fix flaky ScheduleTask_InlineContinuationDoesNotDeadlock test on net481#2957
Conversation
… nested blocking Wait Co-authored-by: martincostello <1439341+martincostello@users.noreply.github.com>
Allow running CI for PRs targeting any branch, not just main.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## increase-deadlock-test-timeout #2957 +/- ##
===============================================================
Coverage 96.15% 96.15%
===============================================================
Files 309 309
Lines 7128 7128
Branches 1005 1005
===============================================================
Hits 6854 6854
Misses 221 221
Partials 53 53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs
Outdated
Show resolved
Hide resolved
…rect deadlock detection with increased timeout Co-authored-by: martincostello <1439341+martincostello@users.noreply.github.com>
- Wait on both tasks with `Task.WaitAll()`. - Suppress `xUnit1031`.
f3b04dd
into
increase-deadlock-test-timeout
There was a problem hiding this comment.
Pull request overview
This PR aims to fix intermittent failures of ScheduleTask_InlineContinuationDoesNotDeadlock on net481 by increasing the test timeout, and it also changes several GitHub Actions workflows to trigger on all pull requests (by removing base-branch filters).
Changes:
- Increased the deadlock-detection test timeout from 1s to 10s and updated the final wait assertion.
- Added a new
xUnit1031pragma suppression around a blocking wait in the test. - Removed
pull_request.branchesfilters from multiple GitHub Actions workflows (broadening when they run).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
test/Polly.Core.Tests/CircuitBreaker/Controller/ScheduledTaskExecutorTests.cs |
Increases timeout and changes how completion is waited for in the deadlock regression test. |
.github/workflows/build.yml |
Removes pull_request base-branch filters, expanding workflow trigger scope. |
.github/workflows/lint.yml |
Removes pull_request base-branch filters, expanding workflow trigger scope. |
.github/workflows/mutation-tests.yml |
Removes pull_request base-branch filters, expanding workflow trigger scope. |
.github/workflows/updater-approve.yml |
Removes pull_request base-branch filters, expanding workflow trigger scope. |
.github/workflows/gh-pages.yml |
Removes pull_request base-branch filters, expanding workflow trigger scope. |
.github/workflows/dependency-review.yml |
Removes pull_request base-branch filters, expanding workflow trigger scope. |
.github/workflows/code-ql.yml |
Removes pull_request base-branch filters, expanding workflow trigger scope. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The
ScheduleTask_InlineContinuationDoesNotDeadlocktest was intermittently failing on net481 CI because the 1-second timeout was too tight. A previous attempted fix inadvertently broke the test's ability to detect the deadlock by moving the blocking wait outside theExecuteSynchronouslycontinuation — meaning the test passed even withoutTaskCreationOptions.RunContinuationsAsynchronously.Details on the issue fix or feature implementation
secondTask.Wait(timeout)stays inside theExecuteSynchronouslycontinuation. This is what actually blocks the background processing thread whenRunContinuationsAsynchronouslyis absent — causing the deadlock the test is designed to catch.#pragma warning disable xUnit1031suppressions: xUnit1031 targetsasync Tasktest methods; it does not apply tovoidsync tests.The test now correctly fails (after the full timeout) when
TaskCreationOptions.RunContinuationsAsynchronouslyis removed from theTaskCompletionSourceinScheduledTaskExecutor, and passes with the current implementation.Confirm the following
Original prompt
This pull request was created from Copilot chat.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.