[Bug] Deadlock during RuntimeBroker.ProcessExit causing desktop application to hang while closing

[mslukebo]

Library version used

4.60.4

.NET version

.NET SDK 8.0.310

Scenario

PublicClient - desktop app

Is this a new or an existing app?

The app is in production, I haven't upgraded MSAL, but started seeing this issue

Issue description and reproduction steps

If my application invokes GetAccountsAsync on my public client application while the application is in the process of closing, a deadlock can be observed on MSAL threads that indefinitely prevent the app from shutting down.

When this scenario happens, the following threads can be observed in a debugger:

Thread A (name: msalruntime.dll!thread_start<unsigned int (__cdecl*)(void *),1>):

[Waiting on lock owned by Thread 3968, double-click or press enter to switch to thread]	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.Module.AddRef(string handleName) Line 33	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.Handle.Handle(bool releaseModule) Line 18	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.MSALRUNTIME_DISCOVER_ACCOUNTS_RESULT_HANDLE.MSALRUNTIME_DISCOVER_ACCOUNTS_RESULT_HANDLE(nint hndl) Line 67	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.API.DiscoverAccountsCallbackCompletion.AnonymousMethod__44_0(nint h) Line 90	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.API.CallbackCompletion<Microsoft.Identity.Client.NativeInterop.MSALRUNTIME_DISCOVER_ACCOUNTS_RESULT_HANDLE>(nint hResponse, nint callbackData, System.Func<nint, Microsoft.Identity.Client.NativeInterop.MSALRUNTIME_DISCOVER_ACCOUNTS_RESULT_HANDLE> convert) Line 116	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.API.DiscoverAccountsCallbackCompletion(nint hResponse, nint callbackData) Line 90	 
[Native to Managed Transition]	 
[Inline Frame] msalruntime.dll!std::_Func_class<void,std::shared_ptr<Msai::SignOutResultInternal> const &>::operator()(const std::shared_ptr<Msai::SignOutResultInternal> &) Line 883	 
msalruntime.dll!Msai::SignOutEventSinkImpl::OnComplete(const std::shared_ptr<Msai::SignOutResultInternal> & signOutResult) Line 17	 
msalruntime.dll!Msai::DiscoverAccountsRequest::FireCallback(const std::shared_ptr<Msai::DiscoverAccountsResultInternal> & result) Line 128	 
msalruntime.dll!Msai::DiscoverAccountsRequest::Execute() Line 100	 
msalruntime.dll!Msai::ThreadPool::ExecuteQueueItemThreadProc(const std::shared_ptr<Msai::BackgroundRequestQueueItem> & queueItem) Line 406	 
msalruntime.dll!Msai::ThreadWorkLoopImpl::WaitForWork() Line 115	 
msalruntime.dll!`anonymous namespace'::ThreadProc(void * lpParameter) Line 20	 
msalruntime.dll!thread_start<unsigned int (__cdecl*)(void *),1>(void * const parameter) Line 97	 
kernel32.dll!BaseThreadInitThunk(unsigned long RunProcessInit, long(*)(void *) StartAddress, void * Argument) Line 77	 
ntdll.dll!RtlUserThreadStart(long(*)(void *) StartAddress, void * Argument) Line 1224

Thread B (name: .NET Finalizer, id: 3968):

ntdll.dll!ZwWaitForSingleObject() Line 268	 
KernelBase.dll!WaitForSingleObjectEx(void * hHandle, unsigned long dwMilliseconds, int bAlertable) Line 1328	 
msalruntime.dll!neosmart::WaitForEvent(neosmart::neosmart_event_t_ * event, unsigned __int64) Line 572	 
msalruntime.dll!Msai::ThreadPool::CancelBackgroundRequests() Line 482	 
msalruntime.dll!Msai::ThreadPool::Stop() Line 207	 
msalruntime.dll!Msai::RequestDispatcherWithPool::Stop() Line 56	 
msalruntime.dll!Msai::AuthenticatorFactoryInternal::Shutdown() Line 541	 
[Managed to Native Transition]	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.API.x64.Shutdown() Line 158	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.Module.RemoveRef(string handleName) Line 60	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.Handle.Dispose(bool disposing) Line 45	 
System.Private.CoreLib.dll!System.Runtime.InteropServices.SafeHandle.Dispose() Line 108	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.Core.ReleaseCallback() Line 175	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.Core.Dispose(bool disposing) Line 163	 
Microsoft.Identity.Client.NativeInterop.dll!Microsoft.Identity.Client.NativeInterop.Core.Dispose() Line 155	 
Microsoft.Identity.Client.Broker.dll!Microsoft.Identity.Client.Platforms.Features.RuntimeBroker.RuntimeBroker.OnProcessExit(object sender, System.EventArgs e) Line 86	 
[Native to Managed Transition]	 
kernel32.dll!BaseThreadInitThunk(unsigned long RunProcessInit, long(*)(void *) StartAddress, void * Argument) Line 77	 
ntdll.dll!RtlUserThreadStart(long(*)(void *) StartAddress, void * Argument) Line 1224 

Notice that thread A is waiting on a lock held by thread B. Thread B is blocking my application from shutting down presumably because OnProcessExit is an event handler for Process.Exited that needs to return before the process exit routine completes. It's not clear from thread B's callstack, but my hypothesis is that the CancelBackgroundRequests is waiting for thread A (a background request on a separate thread) to return.

Verbose MSAL logs: msal_logs.txt

I found this comment on an abandoned pull request that may be related to this scenario.

Expected behavior

I would expect GetAccountsAsync to return an empty collection if it is unable to retrieve accounts due to MSAL being shut down.

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

No response

Solution and workarounds

Thread A above should be provided a cancellation token to cancel the background operation when a shutdown is happening. The thread should cleanly exit when the token is cancelled, which would allow the process to exit. If applicable, an overloaded GetAccountAsync method that accepts a cancellation token to use could be available as well.

I cannot think of any workaround besides simply ensuring my application does not invoke GetAccountsAsync at the wrong time. For some applications, this might be difficult to entirely prevent.