This can return the value now that there is just a single value to return

Updated the suggestion toFreezeStackTrace since the method does not actually perform a deep copy in typical case anymore.

How is this possible?

If I understand the code correctly, the thread safety should be guaranteed by writing thesize field last and reading it first. Is that right? (Whatever it is, it would be nice to have it documented in a comment somewhere.)

I do not think it should be possible to get methods items without corresponding keep alive items here. If it was possible, I suspect there may be situations where the method is not kept alive and thepMethod->IsLCGMethod() check above can crash.

Here is the case when it happens:

• Thread A owns the stack trace and is adding a new entry that requires a keep alive object.
• Thread B wants to update the stack trace too, so it fetches the stack trace array S1 and keep alive array K1 from the exception
• Thread A finds the keep alive array is full, so it creates a larger clone of the previous one, let's call it K2
• Thread A adds the new entry to keep alive array K2
• Thread A adds the new entry to stack trace array S1
• Thread A writes K2 to the exception
• Now thread B creates clones of S1 and K1 (when it read it from the exception, the K2 was not there yet). But S1 already has the new entry added by thread A, which is not covered by K1. Since K1 won't change anymore, we solve this by trimming the S1 at the element where we've found a frame that needs keep alive object and was not covered by K1

However, I have realized you are right that in such case, calling IsLCGMethod on the MethodTable above could crash. For example, if the thread A's exception was collected together with its keep alive array K2 that can be the only thing holding the method alive and if that would happen before the thread B called the IsLCGMethod.
So I need to figure out some other way to handle the situation described above.

I am not sure what this comment is trying to say.

Ah, I've renamed the frozenStackTrace to result and forgotten to update the comment. It is trying to say that the gc.result is not protected after the HELPER_METHOD_FRAME_END

Ok, that's writing manually managed code 101.

(We will want to convert this method to QCALL in near future to get rid of the HELPER_METHOD_FRAME.)

We should use volatile read for the Size. Otherwise, the C++ compiler is free to duplicate the read and use onesize for value for the memcpy and return a different value from the method.

I have thought you were suggesting before that I get rid of the Volatile here, so I am bit confused.

I would:

• Get rid of the volatile local variable. It does not make sense to tell the compiler to issue read/write barriers when reading/writing stack. It is deoptimization.
• Change the read in Size() method to be volatile read, so that it is not reordered. This volatile read is counterpart of volatile write inStackTraceArray::Append (it is not actual volatile write, but MemoryBarrier + regular write that is equivalent in this situation). In general, volatile reads and volatile writes in lock-free algorithm have to come in pairs on producing/consuming sides.

Ok, I have added a commit with that change. I've modified the GetSize / SetSize and added such treatment to the keep alive count accessors too. And removed the MemoryBarrier from the StackTraceArray::Append where it is no longer needed after this change.