From 3f74f4899ab9846c46dfec9ccccb54bdd8d0871c Mon Sep 17 00:00:00 2001 From: mad1081 Date: Fri, 27 Mar 2026 00:05:41 +0500 Subject: [PATCH] Fix UnobservedTaskException in TaskToAsyncResult when APM callback skips EndXxx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the APM pattern, exceptions from the underlying operation are meant to be surfaced exclusively through the End method. However, if the callback passed to Begin does not call End (a valid shutdown pattern), the wrapped Task's exception remains unobserved. When the TaskAsyncResult and its inner Task are garbage-collected, TaskScheduler.UnobservedTaskException fires — a regression from .NET Framework 4.8, which had no Task wrapper. Fix: access task.Exception (marking it observed) before invoking the callback in both the synchronous-completion path and the async continuation path. End still re-throws the exception correctly if it is called. This matches the pattern used in PR #114226 for ResettableValueTaskSource. Fixes #126148 --- .../Threading/Tasks/TaskToAsyncResult.cs | 13 +++- .../TaskToAsyncResultTests.cs | 69 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Threading/Tasks/TaskToAsyncResult.cs b/src/libraries/Common/src/System/Threading/Tasks/TaskToAsyncResult.cs index 086bc40f0cbd2d..99db46ff7a6794 100644 --- a/src/libraries/Common/src/System/Threading/Tasks/TaskToAsyncResult.cs +++ b/src/libraries/Common/src/System/Threading/Tasks/TaskToAsyncResult.cs @@ -128,6 +128,10 @@ internal TaskAsyncResult(Task task, object? state, AsyncCallback? callback) // The task has already completed. Treat this as synchronous completion. // Invoke the callback; no need to store it. CompletedSynchronously = true; + // Observe any fault so that UnobservedTaskException is not raised if End + // is never called. In the APM pattern exceptions are propagated via End; + // End will still rethrow the exception if called. + _ = task.Exception; callback?.Invoke(this); } else if (callback is not null) @@ -138,7 +142,14 @@ internal TaskAsyncResult(Task task, object? state, AsyncCallback? callback) _callback = callback; _task.ConfigureAwait(continueOnCapturedContext: false) .GetAwaiter() - .OnCompleted(() => _callback.Invoke(this)); + .OnCompleted(() => + { + // Observe any fault so that UnobservedTaskException is not raised if End + // is never called. In the APM pattern exceptions are propagated via End; + // End will still rethrow the exception if called. + _ = _task.Exception; + _callback.Invoke(this); + }); } } diff --git a/src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/TaskToAsyncResultTests.cs b/src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/TaskToAsyncResultTests.cs index e293ad8807d3f3..3a8bb490f4a0b7 100644 --- a/src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/TaskToAsyncResultTests.cs +++ b/src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/TaskToAsyncResultTests.cs @@ -152,6 +152,75 @@ public async Task WithFromAsync_Delegate_Roundtrips() tcs.SetResult(); await invoked.Task; } + + [Fact] + public async Task FaultedTask_CallbackDoesNotCallEnd_NoUnobservedTaskException_Async() + { + // Regression test: if the APM callback does not call End, a faulted task must not + // surface via TaskScheduler.UnobservedTaskException. In the APM pattern, exceptions + // are only propagated through End; skipping End (e.g. during shutdown) must be silent. + bool unobservedFired = false; + EventHandler handler = (s, e) => unobservedFired = true; + TaskScheduler.UnobservedTaskException += handler; + try + { + var tcs = new TaskCompletionSource(); + + // Callback intentionally does not call End. + WeakReference weakRef = await Task.Run(() => + { + IAsyncResult ar = TaskToAsyncResult.Begin(tcs.Task, _ => { /* no End */ }, null); + tcs.SetException(new InvalidOperationException("test fault")); + return new WeakReference(ar); + }); + + // Allow the callback continuation to complete, then drop all references. + await Task.Delay(100); + + for (int i = 0; i < 5 && weakRef.IsAlive; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + await Task.Delay(100); + } + + Assert.False(unobservedFired, "UnobservedTaskException should not fire when End is not called in APM callback"); + } + finally + { + TaskScheduler.UnobservedTaskException -= handler; + } + } + + [Fact] + public void FaultedTask_AlreadyCompleted_CallbackDoesNotCallEnd_NoUnobservedTaskException_Sync() + { + // Same regression but for the synchronous-completion path (task already faulted + // when Begin is called, callback fires inline). + bool unobservedFired = false; + EventHandler handler = (s, e) => unobservedFired = true; + TaskScheduler.UnobservedTaskException += handler; + try + { + Task faulted = Task.FromException(new InvalidOperationException("test fault")); + + // Callback intentionally does not call End. + IAsyncResult ar = TaskToAsyncResult.Begin(faulted, _ => { /* no End */ }, null); + Assert.True(ar.CompletedSynchronously); + + ar = null; + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + Assert.False(unobservedFired, "UnobservedTaskException should not fire when End is not called in APM callback"); + } + finally + { + TaskScheduler.UnobservedTaskException -= handler; + } + } } internal sealed class NonTaskIAsyncResult : IAsyncResult