fix(worker): observe detached drain faults per NEXT-04 discipline
Both tasks the detached-lock-wait path starts and discards now carry the file's fault-observing continuation, factored out of ObserveAbandonedFault as ObserveFault so the idiom has one definition. DrainDetachedAsync swallows the drain but its _writeLock.Release() sits in a finally outside that catch, so a Release that ever throws — a SemaphoreFullException from some future double-release regression — had no awaiter and would have surfaced on net48 as TaskScheduler.UnobservedTaskException at finalization instead of an attributable failure. Same for a throw out of OnDetachedLockWaitSettled. Review's Minor (distinguishing a cancelled from a faulted lock wait before tombstoning) is deliberately not taken: a faulted WaitAsync is unreachable here — nothing disposes _writeLock — so the branch would be untestable new logic whose only effect is internal state, the caller already receiving the fault itself from the rethrow. Recorded as a comment at the site instead. edited on macOS, windev verification pending (plan Task 11). Re-ran the net10 scratch harness over WorkerFrameWriter and the writer suite: 0 warnings, 31/31 pass.
This commit is contained in:
@@ -172,6 +172,12 @@ public sealed class WorkerFrameWriter
|
|||||||
// caller still observes cancellation while the frame reaches the wire (documented
|
// caller still observes cancellation while the frame reaches the wire (documented
|
||||||
// above). Rethrow from the wait rather than from the completion, so a claimed frame's
|
// above). Rethrow from the wait rather than from the completion, so a claimed frame's
|
||||||
// canceller is not held behind the very write it is abandoning.
|
// canceller is not held behind the very write it is abandoning.
|
||||||
|
//
|
||||||
|
// The wait ending without the lock IS cancellation in every reachable case — nothing
|
||||||
|
// disposes _writeLock — so the tombstone's TrySetCanceled is honest. A hypothetical
|
||||||
|
// faulted wait would take this same path and label the frame cancelled instead of
|
||||||
|
// faulted; that is internal state only, since the await below rethrows the fault itself
|
||||||
|
// to the caller.
|
||||||
TombstoneIfUnclaimed(frame, cancellationToken);
|
TombstoneIfUnclaimed(frame, cancellationToken);
|
||||||
await lockWait.ConfigureAwait(false);
|
await lockWait.ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
@@ -343,8 +349,23 @@ public sealed class WorkerFrameWriter
|
|||||||
/// <param name="frame">Frame whose completion may fault without an awaiter.</param>
|
/// <param name="frame">Frame whose completion may fault without an awaiter.</param>
|
||||||
private static void ObserveAbandonedFault(PendingFrame frame)
|
private static void ObserveAbandonedFault(PendingFrame frame)
|
||||||
{
|
{
|
||||||
_ = frame.Completion.Task.ContinueWith(
|
ObserveFault(frame.Completion.Task);
|
||||||
task => _ = task.Exception,
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Attaches the NEXT-04 fault-observing continuation to a task this writer starts and then
|
||||||
|
/// discards. Every such task must carry one: with no awaiter, a fault would otherwise reach
|
||||||
|
/// nobody and resurface as <see cref="TaskScheduler.UnobservedTaskException"/> at finalization
|
||||||
|
/// — a detached, unattributable failure long after the code that caused it. The continuation
|
||||||
|
/// runs only on the faulted path and only touches <see cref="Task.Exception"/>, so it can
|
||||||
|
/// never fault itself; it runs inline because an already-faulted task would otherwise pay a
|
||||||
|
/// scheduling hop to do nothing.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="task">Discarded task whose fault would otherwise go unobserved.</param>
|
||||||
|
private static void ObserveFault(Task task)
|
||||||
|
{
|
||||||
|
_ = task.ContinueWith(
|
||||||
|
faulted => _ = faulted.Exception,
|
||||||
CancellationToken.None,
|
CancellationToken.None,
|
||||||
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
|
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
|
||||||
TaskScheduler.Default);
|
TaskScheduler.Default);
|
||||||
@@ -367,11 +388,14 @@ public sealed class WorkerFrameWriter
|
|||||||
/// <param name="lockWait">Outstanding write-lock acquisition the caller has walked away from.</param>
|
/// <param name="lockWait">Outstanding write-lock acquisition the caller has walked away from.</param>
|
||||||
private void DetachLockWait(Task lockWait)
|
private void DetachLockWait(Task lockWait)
|
||||||
{
|
{
|
||||||
_ = lockWait.ContinueWith(
|
// The continuation task is discarded, so it takes the NEXT-04 fault observer: nothing awaits
|
||||||
|
// it, and a throw out of OnDetachedLockWaitSettled would otherwise be an unobserved-task
|
||||||
|
// exception raised at finalization rather than an attributable failure here.
|
||||||
|
ObserveFault(lockWait.ContinueWith(
|
||||||
OnDetachedLockWaitSettled,
|
OnDetachedLockWaitSettled,
|
||||||
CancellationToken.None,
|
CancellationToken.None,
|
||||||
TaskContinuationOptions.None,
|
TaskContinuationOptions.None,
|
||||||
TaskScheduler.Default);
|
TaskScheduler.Default));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
@@ -394,7 +418,11 @@ public sealed class WorkerFrameWriter
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
_ = DrainDetachedAsync();
|
// Discarded, so it takes the NEXT-04 fault observer too. DrainDetachedAsync swallows the drain
|
||||||
|
// itself, but its release sits in a finally outside that catch: a Release that ever throws (a
|
||||||
|
// SemaphoreFullException from some future double-release regression, say) must fail somewhere
|
||||||
|
// attributable rather than at finalization.
|
||||||
|
ObserveFault(DrainDetachedAsync());
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
Reference in New Issue
Block a user