You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Remove global spinlock for EH stacktrace (#103076)
* Remove global spinlock for EH stacktrace
The global spinlock that was used to ensure that stack trace and the
associated dynamic methods array were updated and read atomically.
However, for the new EH, it has shown to cause a high contention in case
many threads were handling exceptions at the same time.
This change replaces the two arrays by one object member in the
exception class. It contains reference to either the byte[] of the
stack trace (when there are no dynamic methods on the stack trace) or an
object[] where the first element contains the stack trace byte[]
reference and the following elements contain what used to be in the
dynamic array. That allows atomic updates and reads of the stack trace
and dynamic method keepalive references without a need of a lock.
The original code was quite convoluted, it was difficult to reason about
and it had some races in it that were hidden behind the global lock. So
I have decided to rewrite the whole thing from scratch.
The way it ensures that it is race free is that whenever it updates the
exception stack trace and the one that's on the exception was created by
a different thread, it creates a deep copy of both the stack trace and
the keepalive array. When making the copy, it also handles a case when
a frame that needs a keepalive entry is on the stack trace part, but the
keepalive array extracted from the exception is stale (the other thread
needed to resize the keepalive array, but not the stack trace). In that
case, the stack trace is trimmed at first such entry found.
Since the case when multiple threads are throwing the same exception and
so they are modifying its stack trace in parallel is pathological
anyways, I believe the extra work spent on creating the clones of the arrays
is a good tradeoff for ensuring easy to reason about thread safety.
I have also removed a dead code path from the
StackTraceInfo::SaveStackTrace.
Finally, since with the previous iteration of this change, a bug in
building the stack trace was found, I have added a coreclr test to
verify stack trace for an exception matches the expectations.
* Fix MUSL build
* Fix x86 build
* Fix several issues
* Missing calls to IsOverflow at few places
* Added a flag on StackTraceElement to indicate that the element needs a
keepalive entry. It removes the need to call IsLCGMethod / Collectible
check on the method table stored in the element and eliminates a
possible problem with the method being collected in one place.
* Returned missing call to StackFrameInfo::Init to the x86 code path
* Removed obsolete comment and code line
* Few changes based on feedback
* Add keep alive items count to the stack trace header.
* Implement the concept of frozen stack traces to eliminate copies in
the ExceptionDispatchInfo storing / restoring exceptions.
* Rename keepalive to keepAlive
* Handle possible array size overflow
In the StackTraceArray::Allocate
* Fix typo
* Change the size / keepAlive fields in stack trace to uint32_t
Plus a build break fix
* Remove SaveStackTracesFromDeepCopy
Also rename GetStackTracesDeepCopy to GetFrozenStackTrace and move the
return argument to return value.
* Remove dummy field and an unused function
* Cleanup based on feedback
* Move the race handling into GetStackTrace only
Plus an unused method removal and a little naming / contract cleanup
* Add VolatileLoad/Store around the size / keep alive count
Also remove the memory barrier from the StackTraceArray::Append since it
is not needed after that change.
* Add comment on why trimming the stack trace by keep alive is needed
I have also realized that when we need to trim, the keepAlive array is
always fully populated, so we don't need to check for cases where there
would be NULL in an entry of the array.
// There are cases where a managed exception does not have a stack trace.
539
539
// These cases are:
@@ -558,6 +558,22 @@ HRESULT ClrDataAccess::DumpManagedExcepObject(CLRDataEnumMemoryFlags flags, OBJE
558
558
// MD this happens.
559
559
StackTraceArray stackTrace;
560
560
exceptRef->GetStackTrace(stackTrace);
561
+
562
+
// The stackTraceArrayObj can be either a byte[] with the actual stack trace array or an object[] where the first element is the actual stack trace array.
563
+
// In case it was the latter, we need to dump the actual stack trace array object here too.
// Given an exception object and deep copied instances of a stacktrace and/or dynamic method array, this method will set the latter in the exception object instance.
0 commit comments