ui: start the main loop just-in-time by mjcheetham · Pull Request #2448 · git-ecosystem/git-credential-manager

mjcheetham · 2026-09-17T09:03:40Z

On macOS, GCM hangs if it shows any UI before an interactive Entra sign-in that uses the broker. Cloning a repository that prompts for a credential and then needs broker authentication is enough to reproduce it. The process never exits and Git waits on it forever.

Why it happens

GCM keeps the process entry thread free so that platform APIs which insist on "thread 1" can be served, and runs the application itself on a second thread. Two features need that thread, for unrelated reasons:

macOS requires UI controls to be created on the entry thread.
The macOS MSAL broker requires a running NSApplication.

Avalonia satisfied the first by posting its main loop to our dispatcher as a job. That job never returns, so from the moment a window was shown the dispatcher's queue stopped being drained. Anything posted afterwards — such as the broker call marshalling itself to the main thread — queued up behind a job that would never finish.

There is a second, less obvious half to this. MSAL decides once per process whether it is a "console app", by testing whether NSApplication is running, and caches the answer for the lifetime of the process:

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

Without NSApplication it requires interactive calls to run on managed thread 1 and then takes that thread over with its own Thread.Sleep(10) polling loop, which cannot coexist with a UI main loop. With NSApplication running it requires neither. So whichever of UI and broker happened to run first silently decided whether the second could work at all — and the "broker first, UI later, broker again" ordering would have frozen the UI even once the queue starvation was fixed.

The fix

Ownership of the entry thread moves into the dispatcher, which now starts the main loop lazily, on the first job posted, and hands the thread over to it.

Needing the main thread therefore implies a running main loop, and because the hand-over completes before any job can be dispatched, every job is guaranteed to run with NSApplication already up. MSAL always sees a GUI app. There is no "start the UI first" call for a caller to forget, and no ordering left to get wrong: UI and broker can now be used in either order, both, or neither.

Startup cost is unchanged for the common case. An invocation that shows no UI and does not use the broker posts no jobs at all: the thread parks on the job queue and shuts down again without ever initialising Avalonia. The silent and default-account authentication probes deliberately stay off the dispatcher for the same reason — only interactive calls decide MSAL's mode, so routing the silent ones through it would pay for Avalonia startup and buy nothing.

Series

Reviewing commit by commit is recommended; each builds and tests green.

1-4 fix pre-existing dispatcher defects that the rework would otherwise have built on. All four predate this work and are independently backportable: jobs that threw left their caller awaiting forever, continuations resumed inline on the dispatcher thread, shutting down before Run() was reached threw on the application thread, and async work handed back a task that completed at its first await rather than at completion.
5 is the change described above.
6 corrects the comment explaining the macOS broker's requirement. It claimed the broker needs the main thread "to display UI", which sends a reader looking for a window parenting problem that does not exist.

Testing

src/Core.Tests/UI/DispatcherTests.cs covers the hand-over and the shutdown paths through a fake main loop: shutting down before Run(), with no work posted, while the loop is running, and with work still outstanding. The last of these pins down the deliberate decision to abandon pending work at shutdown rather than drain it — draining would deadlock the obvious case, where a window is shown without anyone awaiting it and so nothing ever completes the work being waited on.

Notes for reviewers

Tip

Read the "docs/dispatcher.md" documentation (rendered) about how the dispatcher works first as this will help understand the design and operation at a higher level.

Important

The MSAL behaviour this depends on is version-specific. The call sites that evaluate the console/GUI decision are noted in a comment against 4.85.2; DesktopOsHelper.IsMacConsoleApp is worth re-checking on an MSAL upgrade.

One behavioural consequence worth being aware of: the avn_init Trace2 region is now recorded on the main thread rather than on the calling thread, because initialisation no longer has a caller to attribute it to.
Windows and Linux are unaffected. Their brokers never used the dispatcher, and the only other consumer of it is the UI, which needs the main loop regardless.

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>

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>

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. Add overloads that take task-returning work and unwrap it, so the returned task tracks the work to completion. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Copilot

🟡 Changes recommended

Handed-off jobs can remain incomplete if the platform main loop fails before executing them.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Moves platform main-loop ownership into the dispatcher to prevent macOS UI/broker deadlocks.

Changes:

Starts Avalonia lazily when dispatcher work first arrives.
Routes UI and interactive macOS broker work through the dispatcher.
Adds dispatcher lifecycle and shutdown tests.

File summaries

File	Description
`src/Core/UI/IMainLoop.cs`	Defines the platform-loop abstraction.
`src/Core/UI/Dispatcher.cs`	Implements lazy handoff, async jobs, and shutdown.
`src/Core/UI/AvaloniaUi.cs`	Routes window creation through the dispatcher.
`src/Core/UI/AvaloniaMainLoop.cs`	Implements the Avalonia-backed main loop.
`src/Core/Authentication/Entra/EntraAuthentication.PublicClient.cs`	Dispatches interactive macOS broker authentication.
`src/Core.Tests/UI/DispatcherTests.cs`	Tests startup and shutdown behavior.

Review details

Files reviewed: 6/6 changed files
Comments generated: 1
Review effort level: Balanced

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

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, and since the hand-over completes before any job is dispatched, every job is guaranteed to run with NSApplication already up. There is no escalation call for a caller to forget and no ordering left to get wrong. Invocations that need neither UI nor the broker still pay nothing: the thread parks on the job queue 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 itself 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 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 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>

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 the hand-over flips a flag and drains the queue under one lock, why two collections track work, 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 what moves where during the hand-over. Include the rules a caller needs to follow, since the cost of getting them wrong is a hang rather than an obvious failure. Assisted-by: Claude Opus 5 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>

Supersedes [#2448][previous-pr]. 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: ```csharp // 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][dispatcher-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. [previous-pr]: #2448 [dispatcher-guide]: https://github.com/mjcheetham/git-credential-manager/blob/dispatcher-v2/docs/dispatcher.md

mjcheetham added 4 commits September 16, 2026 15:27

mjcheetham requested a review from a team as a code owner September 17, 2026 09:03

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

mjcheetham requested review from dscho and mpysson and a balanced review from Copilot September 17, 2026 09:05

Copilot started reviewing on behalf of mjcheetham September 17, 2026 09:05 View session

mjcheetham added the auth:entra Specific to Microsoft Entra Authentication label Sep 17, 2026

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread src/Core/UI/Dispatcher.cs

mjcheetham added 3 commits September 17, 2026 12:16

mjcheetham force-pushed the macbroker-fix branch from d946a6d to 7d7acdf Compare September 17, 2026 11:31

mjcheetham mentioned this pull request Sep 17, 2026

Add Trace2 instrumentation to Entra authentication #2450

Merged

mjcheetham marked this pull request as draft September 17, 2026 15:33

mjcheetham mentioned this pull request Sep 18, 2026

dispatcher: fix macOS UI and broker auth hangs #2451

Merged

mjcheetham closed this Sep 18, 2026

Navigation Menu

ui: start the main loop just-in-time - #2448

ui: start the main loop just-in-time#2448
mjcheetham wants to merge 8 commits into
git-ecosystem:mainfrom
mjcheetham:macbroker-fix

mjcheetham commented Sep 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

Why it happens

The fix

Series

Testing

Notes for reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mjcheetham commented Sep 17, 2026 •

edited

Loading