The operation of ShouldNotEnter and Enter need to be combined so they are atomic. Currently we could return false from ShouldNotEnter and then Enter a blown away heap.

Easy fix, just enter first then check if we should have entered, also make Enter handle NULL heap.

(In reply to comment #1) > Created an attachment (id=405322) [details] > fix Can you describe the problem in more detail? (Since there's no synchronization here I don't understand how this rearrangement fixes anything, but I'm also not sure I understand the problem that's being addressed.)

Right now this is possible: thread 1 thread 2 ShouldNotEnter returns false OOM happens, destroy heap EnterFrame ctor crash So this change makes that impossible because we call EnterFrame first (which increments the ref count if the GCHeap exists) and then check if we should proceed. Because the heap isn't deleted until the ref count is zero, we're good.

Isnt it possible to again get into the same crash situation? thread1 thread2 checks in the Enterframe ctor if heap is avail destroys heap calls Enter on the heap

My Last comment is not clear: here it is again Isnt it possible to again get into the same crash situation? T1:checks in the Enterframe ctor if heap is present T2: destroy heap T1: calls Enter on the heap

I'm with Ruchi. Unless the lock is lifted to the EnterFrame constructor (at least) I think there will be a race.

Hmm, how do I check that a pointer is not NULL and then perform a lock on a section of memory relative to that pointer atomically? I think we need to lift the lock out of the GCHeap instance to a static variable. I'll submit a new patch.

I think locking will be required in ShouldNotEnter() implementation also. It is also doing a check of gc heap and then gets the status.

This is fixed by the new EnterLock/EnterRelease code

Engineering work item. Marking as verified.