Prevent external-event loss after canceled waits in isolated worker

[wangbill (YunchuWang)]

Summary

This fixes external-event loss in the shared portable .NET worker core, Microsoft.DurableTask.Worker, for the exact sequence from Azure/azure-functions-durable-extension#1676.

Affected consumers include Azure Durable Functions .NET isolated through the gRPC worker integration, standalone portable .NET workers using Worker.Grpc, and hosts built on the same Worker core such as Worker.AzureManaged / Durable Task Scheduler. Client-only and abstractions-only use is not affected because those packages do not dispatch orchestration events. The original #1676 report is against the legacy Azure Functions in-process implementation; that implementation is separate and untouched by this PR.

Failure sequence

• Two differently named waits, event_0 and event_1, share a cancellation token source and race via Task.WhenAny.
• event_0 wins, so the event_1 waiter is canceled/abandoned.
• An activity runs, and event_1 is raised before ContinueAsNew(..., preserveUnprocessedEvents: true).
• The next generation should receive event_1, but the old worker dropped it.

Root cause and fix

The failed TrySetResult on a canceled/abandoned waiter was treated as if the event had been consumed. The fix walks the existing LIFO stack, skips waiters that reject the event, and delivers it to the first live waiter. If none accepts it, the existing buffer/ContinueAsNew forwarding path handles the event.

Deserialization is now lazy and type-aware, with a per-EventType cache for one dispatch call. This preserves the distinct OperationResult entity deserializer, null results, LIFO behavior, and late-event forwarding semantics.

Validation

• Pre-fix: 3/4 regression cases failed.
• Current head targeted TaskOrchestrationContextWrapperTests: 22/22; full Worker.Tests: 137/137.
• Live Azure E2E ran on commit 24956082fed85f8a4a9cdd30cfe6caaccc93cb6c using an Azure-hosted Linux .NET isolated Consumption app with real Azure Storage and an explicit Blob-gated exact sequence: 10/10 unique instances completed with Passed:true, generation 1, and matching first/second payloads. The temporary resource group was deleted and cleanup verified.
• Current head 14b175eadd6924b6fbd26f8f7cb2707ac8da60bb adds review follow-ups (sealing the private nested type and deserialization caching). Those follow-ups were covered by the unit and full Worker.Tests runs; live Azure was not rerun after these non-semantic follow-ups.

Related work

• #508 / #509 changed waiter ordering.
• #711 forwards late events after ContinueAsNew is already scheduled.

Neither covers this exact all-waiters-dead, pre-ContinueAsNew gap.

[Copilot]

🟢 Approval recommended

The core fix is localized, preserves existing semantics (LIFO + buffering/forwarding), and is backed by targeted regression tests, with only minor test-code nits noted.

Pull request overview

This PR fixes a correctness issue in the isolated worker’s external-event delivery path where an event could be unintentionally dropped if the top-of-stack waiter had already been canceled/abandoned (e.g., the losing side of a Task.WhenAny race), ensuring late events are either delivered to a live waiter or fall back to the existing buffering/ContinueAsNew forwarding behavior.

Changes:

• Update external-event completion to skip canceled/completed/abandoned waiters and deliver to the first live waiter (preserving LIFO semantics).
• Extend the internal event-waiter contract so TrySetResult reports whether delivery actually succeeded.
• Add/expand wrapper tests covering canceled-waiter scenarios (including ContinueAsNew preservation) and related scheduling/versioning behaviors.

File summaries

File	Description
src/Worker/Core/Shims/TaskOrchestrationContextWrapper.cs	Adjusts CompleteExternalEvent to walk the waiter stack until a live consumer accepts the event, otherwise buffer/forward as before.
src/Worker/Core/Shims/TaskOrchestrationContextWrapper.EventSource.cs	Changes IEventSource.TrySetResult to return success/failure so canceled/abandoned waiters don’t “consume” events.
test/Worker/Core.Tests/Shims/TaskOrchestrationContextWrapperTests.cs	Adds regression tests for the canceled-waiter event-loss sequence and validates late-event forwarding/buffering behavior.

Review details

Suppressed comments (2)

test/Worker/Core.Tests/Shims/TaskOrchestrationContextWrapperTests.cs:130

• RawInput is marked obsolete (the production code suppresses CS0618 when using it). These assertions will likely introduce CS0618 warnings in the test project; please suppress the warning locally (or assert via reflection to avoid referencing the obsolete type).

This issue also appears on line 442 of the same file.

        innerContext.SentEvents.Should().ContainSingle();
        innerContext.SentEvents[0].InstanceId.Should().Be(wrapper.InstanceId);
        innerContext.SentEvents[0].EventName.Should().Be("Event");
        innerContext.SentEvents[0].EventData.Should().BeOfType<RawInput>().Which.Value.Should().Be("\"payload\"");
        innerContext.LastContinueAsNewInput.Should().Be("new-input");

test/Worker/Core.Tests/Shims/TaskOrchestrationContextWrapperTests.cs:447

• RawInput is marked obsolete (the production code suppresses CS0618 when using it). This assertion will likely introduce CS0618 warnings in the test project; please suppress the warning locally (or assert via reflection to avoid referencing the obsolete type).

        canceledWait.IsCanceled.Should().BeTrue();
        innerContext.SentEvents.Should().ContainSingle();
        innerContext.SentEvents[0].InstanceId.Should().Be(wrapper.InstanceId);
        innerContext.SentEvents[0].EventName.Should().Be("event_1");
        innerContext.SentEvents[0].EventData.Should().BeOfType<RawInput>().Which.Value.Should().Be("\"payload\"");
    }

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

CompleteExternalEvent currently contains a duplicated/unreachable preserve-unprocessed-events conditional block that should be removed to avoid dead code and confusing control flow.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/Worker/Core/Shims/TaskOrchestrationContextWrapper.cs:527

• The buffering/forwarding tail in CompleteExternalEvent has a duplicated preserveUnprocessedEventsOnContinueAsNew check inside the else-branch. The inner if (this.preserveUnprocessedEventsOnContinueAsNew) is unreachable and makes the control flow harder to follow.

        else
        {
            if (this.preserveUnprocessedEventsOnContinueAsNew)
            {
                // ContinueAsNew has already been scheduled with event preservation enabled.

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[wangbill (YunchuWang)]

Live Azure E2E result: B — PR passes; baseline not run.

• Tested PR head 24956082fed85f8a4a9cdd30cfe6caaccc93cb6c in an Azure-hosted Linux .NET isolated Functions Consumption app in East US using a real Azure Storage Durable backend.
• Used an explicit Blob-gated activity to execute the exact #1676 sequence: event_0 wins a shared-cancellation race, the event_1 waiter is canceled/abandoned, event_1 is raised while the activity is pending and before ContinueAsNew, then the gate is released.
• 10/10 unique orchestration instances completed with Passed:true, Generation:1, and matching first/second payloads. No matching runtime errors were observed in the sampled logs.
• The temporary resource group and all test resources were deleted; resource-group deletion was verified. Baseline was not run, so this is validation that the PR behavior passes live Azure, not a live comparative baseline.

[Copilot]

🟢 Approval recommended

The change is narrowly scoped, preserves existing semantics (LIFO/buffering/forwarding), and is backed by targeted regression tests for the reported failure mode.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/Worker/Core/Shims/TaskOrchestrationContextWrapper.cs:503

• CompleteExternalEvent deserializes rawEventPayload before calling TrySetResult, and will repeat this deserialization for each canceled/completed waiter it skips. Since all waiters for the same eventName are required to share the same EventType (enforced in WaitForExternalEvent), consider caching the deserialized value once and reusing it across loop iterations to avoid repeated work when many abandoned waiters exist.

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The implementation change is narrowly scoped, addresses the stated root cause, and is backed by targeted tests that cover the previously failing cancellation/abandonment scenarios.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite