Coalesce JoinableTaskFactory dispatcher posts

[Andrew Arnott (AArnott)]

Repeated main-thread transitions can leave large numbers of already-executed work items in the underlying dispatcher queue, consuming memory and CPU until the dispatcher eventually processes them.

This change adds a private per-factory queue for pending main-thread callbacks. It keeps at most one lower-level synchronization-context message outstanding, removes callbacks that were already executed through another JTF avenue, and reposts one item at a time to preserve dispatcher fairness. The existing virtual posting hook remains intact, so dispatcher-priority and delegating factories retain their scheduling behavior.

Tests cover conservative posting, stale callback removal, dispatcher priority, and delegating factories across the configured target frameworks.

Fixes #1272

Validation

• Validation insertion
• Validation PR

[Copilot]

Pull request overview

This PR updates JoinableTaskFactory to coalesce posts to the underlying SynchronizationContext by maintaining a per-factory pending-callback queue and ensuring at most one “driver” message is outstanding at a time. This reduces memory/CPU overhead from large backlogs of already-executed work items accumulating in dispatcher queues while preserving fairness by reposting one item at a time.

Changes:

• Add a private per-factory queue for pending underlying-synchronization-context callbacks and a single “driver” callback that executes one pending item and reposts as needed.
• Opportunistically trim already-executed callbacks before dequeuing/executing the next item.
• Add unit tests validating conservative posting (one outstanding message) and removal of callbacks already executed via other JTF avenues.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskFactoryTests.cs	Adds tests and a small test factory to validate conservative posting and stale-callback trimming behavior.
src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs	Implements the coalescing queue + driver-post mechanism for underlying SynchronizationContext posting.

---

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs:707

• When the last pending callback is drained, the private Queue instance is left allocated. If the queue ever grows large (the scenario this PR targets), its backing array capacity can remain large even after Count returns to 0, retaining memory indefinitely. Consider releasing (or trimming) the queue when it becomes empty so bursty workloads don’t permanently increase per-factory memory footprint.

            {
                this.RemoveCompletedPendingUnderlyingSynchronizationContextCallbacks();
                postAnotherCallback = this.pendingUnderlyingSynchronizationContextCallbacks?.Count > 0;
                this.underlyingSynchronizationContextCallbackPending = postAnotherCallback;
            }

test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskFactoryTests.cs:344

• If the provided callback throws, callbackCompleted is never completed, so the JoinableTask can hang forever (and the test suite may deadlock). Completing callbackCompleted in a try/catch avoids the hang and correctly propagates the failure.

                this.SwitchToMainThreadAsync().GetAwaiter().OnCompleted(delegate
                {
                    callback();
                    callbackCompleted.SetResult(null);
                });

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs:433

• Potential lost-driver race when PostToUnderlyingSynchronizationContext throws: the code sets underlyingSynchronizationContextCallbackPending=true before actually posting. If the post attempt throws and another thread enqueues callbacks while the flag is still true, that other thread will skip posting and return successfully; after the catch resets the flag to false, the queue can contain pending callbacks with no underlying message to drive draining until another callback is posted later. This also means exceptions from a failing underlying post may no longer propagate consistently to all enqueuers.

                this.RemoveCompletedPendingUnderlyingSynchronizationContextCallbacks();
                this.pendingUnderlyingSynchronizationContextCallbacks.Enqueue(callback);
                postCallback = !this.underlyingSynchronizationContextCallbackPending;
                this.underlyingSynchronizationContextCallbackPending = true;
            }

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskFactoryTests.cs:364

• This test schedules work via SwitchToMainThreadAsync().GetAwaiter().OnCompleted(...) but never calls GetResult() on the awaiter. If the main-thread transition is canceled/faults, the exception is never observed and the test may incorrectly pass or hang. Capture the awaiter and call GetResult() inside the continuation so failures are surfaced reliably.

                this.SwitchToMainThreadAsync().GetAwaiter().OnCompleted(delegate
                {
                    try
                    {
                        callback();
                        callbackCompleted.SetResult(null);
                    }
                    catch (Exception ex)
                    {
                        callbackCompleted.SetException(ex);
                    }
                });

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

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

test/Microsoft.VisualStudio.Threading.Tests/JoinableTaskFactoryTests.cs:503

• maximumPostDepth can be reduced under concurrency: Interlocked.Exchange(ref maximumPostDepth, Math.Max(VolatileRead, depth)) is not an atomic max update, so another thread may have already recorded a larger depth and this write can overwrite it with a smaller value. Even though the current tests likely run single-threaded, this helper becomes unreliable if it’s ever used from multiple threads, potentially masking recursion regressions. Consider updating the maximum using a CompareExchange loop that only ever increases the value.

            int depth = Interlocked.Increment(ref this.currentPostDepth);
            Interlocked.Exchange(ref this.maximumPostDepth, Math.Max(this.MaximumPostDepth, depth));

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

docfx/docs/threading_rules.md:334

• The sample implementation uses this.dispatcher.Post(callback, state), which isn’t a common API surface (e.g., WPF Dispatcher doesn’t have Post). Consider using a compilable example (and match the protected internal accessibility on the override).

protected override void PostToUnderlyingSynchronizationContextCore(
    SendOrPostCallback callback,
    object state)
{
    this.dispatcher.Post(callback, state);
}

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[Lifeng Lu (lifengl)]

looks good to me.

[Lifeng Lu (lifengl)]

I ran a Copilot session to review this PR, and it posted an interesting issue:

[warning] Although  SynchronizationContext.Post  does not universally guarantee  ExecutionContext  flow, this coalescing changes the behavior of contexts that do flow it per post, including  SingleThreadedSynchronizationContext .  MainThreadAwaiter  also deliberately suppresses flow for  UnsafeOnCompleted  but not  OnCompleted .
Since only the first queued callback causes an underlying post, successor posts capture the first driver’s context and later callbacks run directly within that context. A second  OnCompleted  callback may therefore observe the first callback’s  AsyncLocal  values, while an  UnsafeOnCompleted  callback may unexpectedly inherit them.
Could the queue preserve each callback’s captured context or suppressed-flow state, with driver posts suppressing flow? Tests covering safe-to-safe, safe-to-unsafe, and unsafe-to-safe registrations would verify the existing awaiter behavior.

[Andrew Arnott (AArnott)]

Although SynchronizationContext.Post does not universally guarantee ExecutionContext flow, this coalescing changes the behavior of contexts that do flow it per post...

Fixed in f5df5c2. Each queued callback now stores its own captured ExecutionContext (or null when flow was suppressed), while driver posts suppress context flow. The drain reapplies only that callback's context, preserving safe-to-safe, safe-to-unsafe, and unsafe-to-safe registration behavior; regression coverage exercises all three sequences across the supported target frameworks.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.