Skip to content

Commit

Permalink
[iOS][Android] Fix crash in Exception.CaptureDispatchState
Browse files Browse the repository at this point in the history
There is a crash in `Exception.CaptureDispatchState` when called from one thread at the same time another calls into `Exception.RestoreDispatchState`. The
reason for the crash is due to the way we do not update `foreignExceptionFrames` in a thread-safe way.  `foreignExceptionFrames` is used in both methods and can crash when the size changes before the array is copied.

The fix is to lock when reading from and writing to `foreignExceptionFrames`.

Fixes dotnet#70081
  • Loading branch information
Steve Pfister committed Jun 20, 2022
1 parent 41d9c1c commit 890eb82
Showing 1 changed file with 26 additions and 6 deletions.
32 changes: 26 additions & 6 deletions src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public DispatchState(MonoStackFrame[]? stackFrames)

private bool HasBeenThrown => _traceIPs != null;

private readonly object frameLock = new object();

public MethodBase? TargetSite
{
[RequiresUnreferencedCode("Metadata for the method might be incomplete or removed")]
Expand All @@ -74,9 +76,22 @@ internal DispatchState CaptureDispatchState()

if (foreignExceptionsFrames != null)
{
var combinedStackFrames = new MonoStackFrame[stackFrames.Length + foreignExceptionsFrames.Length];
Array.Copy(foreignExceptionsFrames, 0, combinedStackFrames, 0, foreignExceptionsFrames.Length);
Array.Copy(stackFrames, 0, combinedStackFrames, foreignExceptionsFrames.Length, stackFrames.Length);
MonoStackFrame[] combinedStackFrames;
int fefLength;

// Make sure foreignExceptionFrames does not change at this point.
// Otherwise, the Array.Copy into combinedStackFrames can fail due to size differences
//
// See https://github.com/dotnet/runtime/issues/70081
lock(frameLock)
{
fefLength = foreignExceptionsFrames.Length;

combinedStackFrames = new MonoStackFrame[stackFrames.Length + fefLength];
Array.Copy(foreignExceptionsFrames, 0, combinedStackFrames, 0, fefLength);
}

Array.Copy(stackFrames, 0, combinedStackFrames, fefLength, stackFrames.Length);

stackFrames = combinedStackFrames;
}
Expand All @@ -91,9 +106,14 @@ internal DispatchState CaptureDispatchState()

internal void RestoreDispatchState(in DispatchState state)
{
foreignExceptionsFrames = state.StackFrames;

_stackTraceString = null;
// Isolate so we can safely update foreignExceptionFrames and CaptureDispatchState can read the correct values
//
// See https://github.com/dotnet/runtime/issues/70081
lock(frameLock)
{
foreignExceptionsFrames = state.StackFrames;
_stackTraceString = null;
}
}

// Returns true if setting the _remoteStackTraceString field is legal, false if not (immutable exception).
Expand Down

0 comments on commit 890eb82

Please sign in to comment.