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 #70081
  • Loading branch information
Steve Pfister authored and github-actions committed Jun 22, 2022
1 parent 705ec75 commit cb9ab0f
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 @@ -48,6 +48,8 @@ public DispatchState(MonoStackFrame[]? stackFrames)

private bool HasBeenThrown => _traceIPs != null;

private readonly object frameLock = new object();

public MethodBase? TargetSite
{
get
Expand All @@ -72,9 +74,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 @@ -89,9 +104,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 cb9ab0f

Please sign in to comment.