dispatcher: fix macOS UI and broker auth hangs by mjcheetham · Pull Request #2451 · git-ecosystem/git-credential-manager

mjcheetham · 2026-09-18T12:01:07Z

Supersedes git-ecosystem/git-credential-manager#2448. This version uses a single platform work queue and includes the execution-context and Trace2 changes as separate, reviewable commits.

On macOS, GCM hangs if it shows UI before an interactive Entra sign-in that uses the broker. A credential prompt followed by broker authentication can leave Git waiting on a process that never finishes. Fixing that ordering must not break the reverse ordering, or make every GCM invocation pay to start a UI framework.

Why it happens

GCM runs the application on an AppMain thread and keeps the process entry thread available for platform APIs that need it. Two features depend on the main thread and its main loop, for different reasons:

macOS requires UI controls to be created on the entry thread.
The macOS MSAL broker path depends on whether an NSApplication is running.

Avalonia was started by posting its main loop to GCM's dispatcher as a job. That job never returns until shutdown, so the dispatcher's own queue stops being drained as soon as a window is shown. A subsequent broker call marshalled to the main thread queues behind work that will never finish.

There is a second half to the problem. MSAL decides once per process whether it is a console application, and caches the answer:

// DesktopOsHelper.cs, MSAL 4.85.2
private static readonly Lazy<bool> _isMacConsoleApp =
    new Lazy<bool>(() => !LibObjc.IsNsApplicationRunning());

Without a running NSApplication, MSAL requires interactive broker calls to run on managed thread 1 and takes that thread over with its own polling loop. That cannot coexist with the UI main loop. With NSApplication running, it delegates threading to the broker instead.

Fixing queue starvation alone is therefore not enough. If the broker first sees a console application, starting UI later does not change that cached decision. The "broker, then UI, then broker again" ordering would still freeze the UI.

The fix

The dispatcher owns the platform main loop instead of treating it as a job. It starts that loop lazily, on the first request for main-thread work. Avalonia's startup and pumping move behind an IMainLoop contract, implemented by AvaloniaMainLoop.

On successful startup, posting blocks until IMainLoop.Initialize() has made the platform queue available. Each caller then hands its own job directly to that queue. There is no dispatcher-owned pending-work queue to drain or transfer during startup.

The distinction between accepting and executing work matters here: initialization makes posting safe, but a queued job cannot execute until IMainLoop.Run() is pumping. On macOS, that means the job runs with NSApplication already up. The first interactive broker call consequently sees a GUI application, regardless of whether UI or broker authentication was requested first. Callers do not need to remember an explicit startup step.

Two one-shot gates coordinate this: _workRequested wakes the parked main thread, and _mainLoopReady releases callers waiting to post. Lifecycle, failure, and outstanding-job state are protected separately by System.Threading.Lock. Shutdown opens both gates, and failure also releases callers waiting for readiness. A caller must inspect the outcome rather than assume that waking means initialization succeeded.

Dispatched operations own their caller-facing completion tasks, with RunContinuationsAsynchronously applied directly to those tasks. Async operations remain tracked after their first yield, so reported loop or posting failures can fault those operations rather than leave callers waiting indefinitely. The work's result, exceptions, and cancellation information are preserved.

The common fast path still avoids starting Avalonia. An invocation that posts no main-thread work parks and shuts down without initializing the UI framework. Silent and default-account authentication attempts deliberately remain off the dispatcher: they do not make MSAL's interactive broker console/GUI decision, so starting Avalonia for them would buy nothing.

Context and tracing

Handing a job to another thread should not discard the posting caller's ambient state. The dispatcher captures and restores that caller's ExecutionContext, preserving values such as AsyncLocal<T> and Activity.Current.

Trace2 deliberately needs different attribution. The entry thread now starts the main loop and emits events of its own, so AppMain gets a distinct Trace2 thread context. Dispatched work uses the dispatcher's captured Trace2 context, applied inside the restored execution context so it is not overwritten by the caller's AsyncLocal state. The caller is also recorded as a data event to preserve the link between the two.

This makes the startup and main-thread work distinguishable from the application work that requested it, rather than reporting both as the same thread or losing the caller's other ambient state.

Series

The 12 commits are arranged for commit-by-commit review:

Commits	Purpose
1	Fold the untyped job into the generic implementation so the following fixes have one execution path to maintain.
2-6	Fix job exception propagation, inline task continuations, premature completion of asynchronous work, shutdown before `Run()`, and the distinction between stopping and stopped.
7	Introduce lazy main-loop ownership, the single-queue posting model, startup gates, and failure handling.
8	Correct the explanation of the macOS broker requirement and its cached console/GUI decision.
9-11	Separate the application's Trace2 context, expose a context handle, and preserve the caller's execution context while attributing dispatched work correctly.
12	Document the dispatcher design, thread interactions, lifecycle, and caller rules.

The preparatory fixes address existing dispatcher problems, rather than introducing them as part of the main-loop change and repairing them later. In particular, a job that throws must not leave its caller waiting forever, and an asynchronous delegate must return a task representing the whole operation, not just the part before its first await.

Coverage

The dispatcher cases in src/Core.Tests/UI/DispatcherTests.cs use a fake main loop so startup, failure, and shutdown can be controlled without initializing Avalonia or requiring a real process entry thread. They cover:

Work requested before Run(), concurrent first callers, and later or re-entrant posts once initialization completes.
Accepting a job before the main loop pumps, without executing it early.
Initialization, main-loop, and posting failures, including preserving the first failure and skipping callbacks whose jobs have already been faulted.
Loop or posting failures after an async job yields, without stranding its caller.
Shutdown before Run(), with no work, during initialization, while running, and with work still pending.
Preserving the posting caller's ambient state on both cold and warm paths and across await.

These cases exercise the dispatcher contract; they do not stand in for the native UI/broker integration on macOS.

Notes for reviewers

Tip

Read the dispatcher design guide first. It includes diagrams of the thread interaction, posting path, startup, and lifecycle.

Important

The MSAL behavior described here is version-specific. The broker comment is based on MSAL 4.85.2; DesktopOsHelper.IsMacConsoleApp and the paths that evaluate it should be revisited on an MSAL upgrade.

Posting can block during startup, including calls named InvokeAsync. It is the work's completion that is asynchronous; callers must first wait for a queue that can accept it.
Shutdown abandons pending work rather than draining it. The application must await anything it cares about before shutting the dispatcher down. Draining would hang when a window is left open with nobody awaiting or closing it.
The motivating failure is macOS-specific, but the shared changes are cross-platform. Windows and Linux broker calls still do not marshal through this dispatcher; the dispatcher fixes, UI main-loop ownership, and Trace2 attribution changes also apply on those platforms.

The untyped DispatcherJob and the generic DispatcherJob<TResult> differ only in that one has no result to hand back. Keeping both means every change to how a job runs - how it completes, how it fails, what context it runs in - has to be made and reviewed twice, then kept in step by hand. Express the untyped case as a job with a null result instead. Callers awaiting the untyped overload still get a plain Task, so nothing outside the dispatcher can tell the difference. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

A dispatcher job that threw left its TaskCompletionSource uncompleted, so a caller awaiting InvokeAsync waited forever. The exception then unwound the queue loop and tore down the dispatcher thread with it, so no further main thread work could run either. Complete the task with the exception instead, so failures surface where the work was requested rather than on whichever loop happens to be pumping the dispatcher thread. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

TaskCompletionSource runs its continuations synchronously by default, so a caller awaiting InvokeAsync resumes inline on the dispatcher thread, inside the loop that is meant to be draining the job queue. Whatever the caller does next - including blocking - delays every other job posted to the main thread. Ask for asynchronous continuations so the dispatcher thread returns to pumping as soon as the job itself is done. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Copilot

Copilot review overview

🟡 Changes recommended

Async jobs can escape failure tracking and permit continuations to execute inline on the main-loop thread.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 · 2

Open (4)

Unwrap bypasses asynchronous continuations on the caller-facing task · New
Async jobs are removed before completion, causing hangs on loop failure · New
Complete the sentence by naming the requirement · New
Correct “read” to “ready” · New

What changed in this PR

Introduces a lazily initialized platform main loop to prevent macOS UI and broker authentication hangs while preserving the no-UI fast path.

Changes:

Reworks dispatcher startup, lifecycle, failure handling, and context propagation.
Routes Avalonia and interactive macOS broker work through the dispatcher.
Adds dispatcher tests and architecture documentation.

File	Description
`src/git-credential-manager/Program.cs`	Separates AppMain Trace2 attribution.
`src/Core/UI/IMainLoop.cs`	Defines the platform main-loop contract.
`src/Core/UI/Dispatcher.cs`	Implements lazy startup and job dispatch.
`src/Core/UI/AvaloniaUi.cs`	Routes UI work through the dispatcher.
`src/Core/UI/AvaloniaMainLoop.cs`	Implements the Avalonia main loop.
`src/Core/Tracing/Trace2.cs`	Exposes reusable Trace2 context handles.
`src/Core/Authentication/Entra/EntraAuthentication.PublicClient.cs`	Dispatches macOS interactive broker authentication.
`src/Core.Tests/UI/DispatcherTests.cs`	Covers dispatcher lifecycle and failure paths.
`docs/dispatcher.md`	Documents dispatcher behavior and constraints.
`docs/architecture.md`	Links the dispatcher design into architecture guidance.

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

Passing an async lambda to InvokeAsync bound to the plain Func<T> overload with T inferred as Task, so the returned task completed when the work first yielded rather than when it finished. The broker call site had to notice that and await twice to get the real result. Anyone who missed it got a task that completed early. Give task-returning work explicit overloads and jobs that own their final completion sources. The caller then observes the whole operation, including its original exceptions and cancellation information. The final task must also retain the asynchronous-continuation policy of synchronous jobs. Unwrapping an outer task does not carry that policy to the inner task or the proxy returned to the caller, so completing async work could otherwise run unrelated application code inline on the dispatcher thread. Keep completion handling with the job rather than constructing separate success and failure callbacks at each public entry point. Synchronous and asynchronous jobs share the execution and exception boundary, while each variant completes the caller's task only when its own work is done. Assisted-by: Claude Opus 5 Assisted-by: GPT-6 Astra Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Program.Main starts the application thread and only then runs the dispatcher, so the application thread can reach shutdown before the main thread has reached Run. Shutdown treated that as misuse and threw, which would have surfaced as an unhandled exception on the application thread for an invocation that did nothing wrong - just one that finished unusually quickly. Accept it instead, and have Run return immediately when it finds the dispatcher already stopping. Neither thread has to win the race. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

The state machine declared a Stopped state that nothing ever entered, so every branch handling it was unreachable and the dispatcher could not tell "shut down before Run was reached" from "Run has already returned". Those need to be told apart. Run tolerates the first because the application thread can legitimately finish before the main thread gets that far, but the second is a caller trying to reuse a dispatcher whose thread has already been released, and used to be accepted in silence. Enter Stopped when Run is about to return, and reject running again. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Core concurrency and native main-loop behavior require human validation, with Trace2 coverage and documentation issues still unresolved.

Review effort: Balanced
Findings: 4

Open (4)

Clarify shutdown does not drain outstanding work · New
Limit deadlock warning to synchronous blocking · New
Clarify shutdown may leave async operations incomplete · New
Test Trace2 attribution and context restoration · New

Resolved since last review (4)

Async jobs are removed before completion, causing hangs on loop failure
Unwrap bypasses asynchronous continuations on the caller-facing task
Correct “read” to “ready”
Complete the sentence by naming the requirement

Showing UI and using the macOS MSAL broker both need the process entry thread, for different reasons: macOS requires UI controls to be created there, and the broker requires a running NSApplication. Avalonia supplied the former by running its main loop as a dispatcher job, which never returns. Anything posted afterwards queued up behind it and never ran, so main thread work that followed showing a window deadlocked. MSAL makes the ordering matter a second time. It decides once per process whether it is a "console app" by testing whether NSApplication is running, and caches that answer for the lifetime of the process. Without NSApplication it requires the interactive broker call to run on managed thread 1 and then seizes that thread with its own polling loop, which cannot coexist with Avalonia's. With NSApplication running it requires neither. So whichever of the two ran first silently decided whether the second could work at all. Move the main loop into the dispatcher and start it lazily, on the first job posted. Needing the main thread now implies a running main loop: there is no escalation call for a caller to forget, and no ordering left to get wrong. Posting is what waits, rather than the work. A caller blocks until the main loop is up and then hands its own job straight to it, so the dispatcher never holds work that nothing is pumping, and every job is guaranteed to start with NSApplication already running. The first caller therefore pays to initialise Avalonia, which is fair enough - it is the one that asked for the main thread. Posting from the dispatcher thread before the loop exists is rejected rather than left to deadlock, since that thread is the only one that could release the wait. Invocations that need neither UI nor the broker still pay nothing: the thread parks until somebody asks for it, and shuts down again without ever initialising Avalonia. Shutting down abandons work that is still outstanding, which is worth being deliberate about. Draining instead would look tidier but deadlocks the obvious case: a window shown without anyone awaiting it stays open, so nothing would ever complete the work being waited for. The Avalonia bootstrap moves behind IMainLoop so that the dispatcher carries no UI framework dependency and the hand-over stays testable with a fake. The existing shutdown tests are moved on to that fake so they can also assert that the fast path never starts the loop, and tests are added for the hand-over, for posting from the wrong thread, and for abandoned work. One consequence of the move is that the avn_init trace region is now recorded on the main thread rather than on the calling thread, since initialisation no longer has a caller to attribute it to. Assisted-by: Claude Opus 5 Assisted-by: GPT-6 Astra Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

The comment claimed the broker needs the main thread "to display UI", which sends anyone reading it looking for a window parenting problem. The real constraint is that the macOS broker needs a running NSApplication, and that MSAL decides once per process whether it has one and caches that answer. Without NSApplication it requires interactive calls to run on managed thread 1 and then takes that thread over with its own polling loop. Record why dispatching is what avoids that, why the silent attempts above deliberately do not dispatch, and which MSAL version the reasoning was checked against, since an upgrade could move the decision to another code path. The variable carrying the platform check is dropped: it read as though the main thread were a hard requirement of the call, when it is really how we guarantee NSApplication is up. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

The application thread deliberately ran without a Trace2 thread scope so that its events were attributed to "main". That was accurate enough when it was the only thread doing anything: the main thread merely served queued work, and attributing that work to the thread that asked for it was more useful than naming the thread it happened to run on. That is no longer true. The main thread now starts and runs the platform main loop, and emits events of its own while doing so. Both threads reported as "main", leaving no way to tell the two apart in a trace. Give the application thread its own context so they can be. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Trace2 attributes each event to a logical thread, and until now the only way to change that attribution was UseMainContext, which could restore exactly one context - the process one - and had no callers left. What is actually needed is the general case: capture whichever context a thread is currently reporting as, and apply it somewhere else. The dispatcher runs work on behalf of other threads and needs precisely that. Replace it with a matching get/set pair over an opaque Trace2Context handle. The type carries no public surface of its own, so callers can only pass it back to Trace2, and it stays free to grow internal state without becoming API. Setting is a no-op before initialization or for a null handle, so a context captured early can be restored without a guard. StartThread remains the right tool for genuinely new logical threads; this is for work that continues an existing one somewhere else. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Work handed to the dispatcher runs on the main thread, and picked up whatever ambient state that thread happened to be carrying rather than the caller's. Nothing flowed by AsyncLocal survived the crossing: Activity.Current, and anything else a caller had established around the call, were silently absent inside the job. Neither the platform main loop nor a dispatcher of our own restores it for us. Capture the caller's execution context when the job is created and run the work inside it. A caller that has deliberately suppressed flow captures nothing, and gets the old behaviour. Trace2 has to be carved out of that, because it records which thread work ran on rather than which thread wanted it. Restoring the caller wholesale would have reported main thread work - showing a window, driving the macOS broker - as AppMain, and folded the time it took into whichever region the caller had open. So the dispatcher's own Trace2 context is applied on top, with the caller recorded as data so the link back is not lost. The two interact, which is worth spelling out: the Trace2 switch has to happen inside the restored execution context, since restoring replaces the whole AsyncLocal map and would otherwise shadow a switch made around it. For the same reason the dispatcher's context is captured when it is created rather than read when a job runs - by then the caller's context is in place, and would be the one observed. Assisted-by: Claude Opus 5 Assisted-by: GPT-6 Astra Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

The dispatcher carries a lot of load-bearing subtlety that is hard to recover from the code alone: why the main loop starts on the first job rather than on request, why posting is what blocks rather than the work, why a job is handed to the main loop by its own caller, why two flags gate the rendezvous, and why shutdown abandons outstanding jobs instead of draining them. Write it down, with diagrams for the thread interaction, the state machine, how work is routed, and how the main loop gets started. Document the context handling too, since the execution context and the Trace2 context are deliberately moved in opposite directions and the ordering between them matters. Include the rules a caller needs to follow, since the cost of getting them wrong is a stall or a rejected post rather than an obvious failure. Assisted-by: Claude Opus 5 Assisted-by: GPT-6 Astra Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

dscho

Wow, what a big one. It looks good to me, and I think it will dramatically improve the user experience as well as make debugging easier (with traces that allow following the call path). Great job!

**Requires [#2451](#2451) be merged first!** Add Trace2-based instrumentation around all aspects of Entra authentication. Entra has the most branching of any authentication path in GCM: broker or not, silent or interactive, and within interactive one of three modes - each selected by some combination of user setting, stored preference, platform support and runtime availability. When someone reports that authentication did something unexpected, the answer is almost always one of those decisions, and none of them left a trace that could be correlated with timings.

mjcheetham added 3 commits September 17, 2026 18:30

mjcheetham mentioned this pull request Sep 18, 2026

Add Trace2 instrumentation to Entra authentication #2450

Merged

mjcheetham added gui Specific to graphical user interface controls entra:broker Related to the authentication broker for Entra Authentication platform:osx Specific to the macOS platform labels Sep 18, 2026

mjcheetham requested review from dscho and mpysson September 18, 2026 12:04

mjcheetham marked this pull request as ready for review September 18, 2026 12:05

mjcheetham requested a review from a team as a code owner September 18, 2026 12:05

mjcheetham requested a balanced review from Copilot September 22, 2026 09:35

Copilot started reviewing on behalf of mjcheetham September 22, 2026 09:35 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread src/Core/UI/Dispatcher.cs Outdated

Comment thread src/Core/UI/Dispatcher.cs Outdated

Comment thread docs/dispatcher.md Outdated

Comment thread src/Core/UI/Dispatcher.cs Outdated

mjcheetham added 3 commits September 22, 2026 12:36

mjcheetham force-pushed the dispatcher-v2 branch from 4ea38f6 to 41e4dda Compare September 22, 2026 13:34

mjcheetham requested a balanced review from Copilot September 22, 2026 13:48

Copilot started reviewing on behalf of mjcheetham September 22, 2026 13:49 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread docs/dispatcher.md Outdated

Comment thread docs/dispatcher.md Outdated

Comment thread src/Core/UI/Dispatcher.cs Outdated

Comment thread src/Core/UI/Dispatcher.cs

mjcheetham added 6 commits September 22, 2026 15:21

mjcheetham force-pushed the dispatcher-v2 branch from 41e4dda to e4ede93 Compare September 22, 2026 14:24

dscho approved these changes Sep 23, 2026

View reviewed changes

mjcheetham merged commit ca3dd9b into git-ecosystem:main Sep 23, 2026
27 checks passed

mjcheetham deleted the dispatcher-v2 branch September 23, 2026 11:11

Navigation Menu

dispatcher: fix macOS UI and broker auth hangs - #2451

dispatcher: fix macOS UI and broker auth hangs#2451
mjcheetham merged 12 commits into
git-ecosystem:mainfrom
mjcheetham:dispatcher-v2

mjcheetham commented Sep 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dscho left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mjcheetham commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why it happens

The fix

Context and tracing

Series

Coverage

Notes for reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dscho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mjcheetham commented Sep 18, 2026 •

edited

Loading