Skip to content

Commit 3230699

Browse files
Make ExceptionHandling.RaiseAppDomainUnhandledExceptionEvent() thread safe. (#117844)
* Make the ExceptionHandling.RaiseAppDomainUnhandledExceptionEvent() API thread safe. The RaiseAppDomainUnhandledExceptionEvent() API is permitted to be called by multiple threads but only one should be triggering the event handlers. --------- Co-authored-by: Jan Kotas <[email protected]>
1 parent ebde052 commit 3230699

File tree

3 files changed

+44
-10
lines changed

3 files changed

+44
-10
lines changed

src/libraries/System.Private.CoreLib/src/System/AppContext.cs

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,28 +82,50 @@ public static void SetData(string name, object? data)
8282
internal static event EventHandler<FirstChanceExceptionEventArgs>? FirstChanceException;
8383
#pragma warning restore CS0067
8484

85+
private static ulong s_crashingThreadId;
86+
8587
#if NATIVEAOT
8688
[System.Runtime.RuntimeExport("OnUnhandledException")]
8789
#endif
8890
internal static void OnUnhandledException(object e)
8991
{
92+
ulong currentThreadId = Thread.CurrentOSThreadId;
93+
ulong previousCrashingThreadId = Interlocked.CompareExchange(ref s_crashingThreadId, currentThreadId, 0);
94+
if (previousCrashingThreadId == 0)
95+
{
9096
#if NATIVEAOT
91-
RuntimeExceptionHelpers.SerializeCrashInfo(System.Runtime.RhFailFastReason.UnhandledException, (e as Exception)?.Message, e as Exception);
97+
RuntimeExceptionHelpers.SerializeCrashInfo(System.Runtime.RhFailFastReason.UnhandledException, (e as Exception)?.Message, e as Exception);
9298
#endif
93-
if (UnhandledException is UnhandledExceptionEventHandler handlers)
94-
{
95-
UnhandledExceptionEventArgs args = new(e, isTerminating: true);
96-
foreach (UnhandledExceptionEventHandler handler in Delegate.EnumerateInvocationList(handlers))
99+
if (UnhandledException is UnhandledExceptionEventHandler handlers)
97100
{
98-
try
99-
{
100-
handler(/* AppDomain */ null!, args);
101-
}
102-
catch
101+
UnhandledExceptionEventArgs args = new(e, isTerminating: true);
102+
foreach (UnhandledExceptionEventHandler handler in Delegate.EnumerateInvocationList(handlers))
103103
{
104+
try
105+
{
106+
handler(/* AppDomain */ null!, args);
107+
}
108+
catch
109+
{
110+
}
104111
}
105112
}
106113
}
114+
else
115+
{
116+
if (s_crashingThreadId == previousCrashingThreadId)
117+
{
118+
Environment.FailFast("OnUnhandledException called recursively");
119+
}
120+
121+
// If we are already in the process of handling an unhandled
122+
// exception, we do not want to raise the event again. We wait
123+
// here while the other thread raises the unhandled exception.
124+
// Waiting is important because it is possible upon returning, this thread
125+
// could call some rude abort method that would terminate the process
126+
// before the other thread finishes raising the unhandled exception.
127+
Thread.Sleep(-1);
128+
}
107129
}
108130

109131
internal static void OnProcessExit()

src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionHandling.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ public static void SetUnhandledExceptionHandler(Func<Exception, bool> handler)
4848
/// event and then return.
4949
///
5050
/// It will not raise the the handler registered with <see cref="SetUnhandledExceptionHandler"/>.
51+
///
52+
/// This API is thread safe and can be called from multiple threads. However, only one thread
53+
/// will trigger the event handlers, while other threads will wait indefinitely without raising
54+
/// the event.
5155
/// </remarks>
5256
public static void RaiseAppDomainUnhandledExceptionEvent(object exception)
5357
{

src/tests/baseservices/exceptions/RaiseAppDomainUnhandledExceptionEvent/RaiseEvent.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33
using System;
4+
using System.Runtime.CompilerServices;
45
using System.Runtime.ExceptionServices;
56
using TestLibrary;
67
using Xunit;
@@ -19,7 +20,14 @@ public HandlerRegistration(UnhandledExceptionEventHandler handler)
1920
public void Dispose()
2021
{
2122
AppDomain.CurrentDomain.UnhandledException -= _handler;
23+
24+
// See usage of s_crashingThreadId in the ExceptionHandling class.
25+
// This is to ensure that the static field is reset after the test.
26+
GetCrashingThreadId(null) = 0;
2227
}
28+
29+
[UnsafeAccessor(UnsafeAccessorKind.StaticField, Name = "s_crashingThreadId")]
30+
private static extern ref ulong GetCrashingThreadId([UnsafeAccessorType("System.AppContext")]object? obj);
2331
}
2432

2533
[ThreadStatic]

0 commit comments

Comments
 (0)