Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

Problem

use t = Async.StartAsTask(...) creates a race condition: F# disposes the Task at scope exit, but Task.Dispose() throws InvalidOperationException if called before the task reaches a terminal state. Under CI load, this manifests as intermittent failures in StartAsTaskCancellation and other async tests.

Changes

Replaced use with let for all Task bindings from Async.StartAsTask and Async.StartImmediateAsTask in 10 test methods:

- use t : Task<unit> = Async.StartAsTask(a, cancellationToken = cts.Token)
+ let t : Task<unit> = Async.StartAsTask(a, cancellationToken = cts.Token)

Affected tests in AsyncType.fs:

  • CreateTask
  • StartAsTaskCancellation (primary flaky test)
  • ExceptionPropagatesToTask
  • CancellationPropagatesToTask
  • CancellationPropagatesToGroup
  • CreateImmediateAsTask
  • StartImmediateAsTask
  • ExceptionPropagatesToImmediateTask
  • CancellationPropagatesToImmediateTask
  • CancellationPropagatesToGroupImmediate

Task disposal is a no-op in modern .NET except for releasing an internal ManualResetEventSlim that tests never access.

Original prompt

Problem

The StartAsTaskCancellation test in AsyncType.fs is flaky in CI, failing with:

System.InvalidOperationException : A task may only be disposed if it is in a completion state (RanToCompletion, Faulted or Canceled).
   at System.Threading.Tasks.Task.Dispose(Boolean disposing)
   at System.Threading.Tasks.Task.Dispose()
   at FSharp.Core.UnitTests.Control.AsyncType.StartAsTaskCancellation()

The root cause is the use t : Task<unit> = Async.StartAsTask(...) pattern — F# calls t.Dispose() when the binding goes out of scope, but under CI load the task may not have reached a terminal state yet. Task.Dispose() throws InvalidOperationException if the task isn't completed.

This same risky pattern (use t = Async.StartAsTask ...) appears throughout the file and in related test files, making multiple tests susceptible to the same flaky failure.

Required Changes

1. Fix StartAsTaskCancellation (primary failure)

In tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/AsyncType.fs, line 170:

use t : Task<unit> = Async.StartAsTask(a, cancellationToken = cts.Token)

Change use to let. Task.Dispose() is largely a no-op in modern .NET (it only releases an internal WaitHandle if one was allocated, which is rare). Removing the use eliminates the race where Dispose is called before the task completes. Do the same for all other use t = Async.StartAsTask ... and use t = Async.StartImmediateAsTask ... bindings in this file where the task may not be in a completed state when the use scope ends, particularly in cancellation test scenarios.

The affected test methods in AsyncType.fs include:

  • StartAsTaskCancellation (line 170) — the primary flaky test
  • CreateTask (line 156)
  • ExceptionPropagatesToTask (line 238)
  • CancellationPropagatesToTask (line 253)
  • CancellationPropagatesToGroup (line 273)
  • CreateImmediateAsTask (line 290)
  • StartImmediateAsTask (line 299)
  • ExceptionPropagatesToImmediateTask (line 310)
  • CancellationPropagatesToImmediateTask (line 325)
  • CancellationPropagatesToGroupImmediate (line 346)
  • TaskAsyncValue (line 361)
  • TaskAsyncValueException (line 407)
  • TaskAsyncValueCancellation (line 421 — already uses let for t1, but use t for the inner task)
  • NonGenericTaskAsyncValue (line 443)
  • NonGenericTaskAsyncValueException (line 453)
  • NonGenericTaskAsyncValueCancellation (line 467 — same pattern)

For tests where the task IS always completed before scope exit (e.g., CreateTask which calls waitASec and asserts IsCompleted), use is technically safe but should still be changed to let for consistency and to prevent future regressions.

2. Review and fix similar patterns in related test files

Check these files for the same use t = ...Task... pattern and apply the same fix:

  • tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/AsyncModule.fs
  • tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/Cancellation.fs

3. Do NOT change test logic or assertions

The fix is specifically about replacing use with let for Task bindings. Do not change the test logic, timing values, assertions, or control flow. The tests should continue to verify the same behavior — we're just removing the unsafe disposal pattern.

Key Technical Context

  • Task.Dispose() throws InvalidOperationException if the task is not in a completion state (RanToCompletion, Faulted, or Canceled)
  • In modern .NET, Task.Dispose() is rarely needed — it only frees an internal ManualResetEventSlim used by Task.WaitHandle, which most code never accesses
  • The F# use keyword calls Dispose() unconditionally at scope exit
  • Under CI load, timing-sensitive cancellation tests may exit scope before the task transitions to a terminal state

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix flaky StartAsTaskCancellation test in AsyncType.fs Fix flaky AsyncType tests by removing premature Task disposal Feb 11, 2026
Copilot AI requested a review from T-Gro February 11, 2026 20:27
@github-actions
Copy link
Contributor

✅ No release notes required

@T-Gro T-Gro marked this pull request as ready for review February 12, 2026 13:12
@T-Gro T-Gro requested a review from a team as a code owner February 12, 2026 13:12
@T-Gro T-Gro requested a review from abonie February 12, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants