556520 - Deadlock during memoryStatusChange notifications

Status: VERIFIED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P2)

Description

[Brent Getlin]

GCHeap cannot hold on to spinLock when it leaves the scope of MMGC.  This is because any other lock acquired outside of memory manager could trigger a deadlock between threads since the MMGC lock is "supposed to" be the top lock ever owned.

Here's the callstack from one such case:
Thread 1:  owns Platform sound thread lock, attempts to get MMGC lock 
                Flash.ocx!VMPI_lockAcquire(vmpi_spin_lock_t * lock=0x0b0f8614)  Line 266 + 0x12 bytes            C++ 
                Flash.ocx!MMgc::GCAcquireSpinlock::GCAcquireSpinlock(vmpi_spin_lock_t * spinlock=0x0b0f8614)  Line 64 + 0xb bytes     C++ 
                Flash.ocx!MMgc::GCHeap::Leave()  Line 2207     C++ 
                Flash.ocx!MMgc::EnterFrame::~EnterFrame()  Line 1967               C++ 
                Flash.ocx!PlatformSoundMix::SoundThreadLocked(bool & blockAvailable=true)  Line 79 + 0x8 bytes       C++ 
                Flash.ocx!PlatformSoundMix::SoundThread()  Line 102  C++ 
                Flash.ocx!PlatformSoundMix::SoundThreadFunc(void * param=0x0ca8f010)  Line 38       C++ 


Thread 2: Owns GCHeap spinlock, attempts to get sound thread lock 
                Flash.ocx!_PlatformCriticalSectionBase_::PlatformEnter()  Line 252 + 0xf bytes  C++ 
                Flash.ocx!PlatformCriticalSection::PlatformEnter()  Line 1061      C++ 
                Flash.ocx!CoreCriticalSectionBase::Enter()  Line 26 + 0xf bytes    C++ 
                Flash.ocx!PlatformGlobals::LockAudioCallback()  Line 107 + 0x1c bytes   C++ 
                Flash.ocx!BufferedPlayQueue::Clear(bool streamBegin=true, bool isSmartBuffering=false)  Line 8873    C++ 
                Flash.ocx!NetStream::InitBufferedQueue(unsigned int bufTime=4500, bool reset=true)  Line 5449          C++ 
                Flash.ocx!NetStream::Close()  Line 899  C++ 
                Flash.ocx!NetStream::Destroy()  Line 761             C++ 
                Flash.ocx!avmplus::NetStreamPlus::~NetStreamPlus()  Line 1555             C++ 
                Flash.ocx!avmplus::NetStreamPlus::`scalar deleting destructor'()  + 0x16 bytes  C++ 
>             Flash.ocx!MMgcDestructTaggedScalarChecked<NetStream>(NetStream * mem=0x1fbba010)  Line 291 + 0x10 bytes     C++ 
                Flash.ocx!NetStreamDestroyProc(ScriptObject * object=0x1a096360, unsigned int userData=532389904)  Line 473 + 0x9 bytes             C++ 
                Flash.ocx!ScriptObject::~ScriptObject()  Line 14190 + 0x19 bytes                C++ 
                Flash.ocx!ScriptObject::`scalar deleting destructor'()  + 0x16 bytes            C++ 
                Flash.ocx!MMgc::GCAlloc::Finalize()  Line 840 + 0x10 bytes           C++ 
                Flash.ocx!MMgc::GC::Finalize()  Line 1531            C++ 
                Flash.ocx!MMgc::GC::Sweep()  Line 1668             C++ 
                Flash.ocx!MMgc::GC::FinishIncrementalMark(bool scanStack=false)  Line 3250  C++ 
                Flash.ocx!MMgc::GC::Collect(bool scanStack=false)  Line 1128    C++ 
                Flash.ocx!MMgc::GC::memoryStatusChange(MMgc::_MemoryStatus __formal=kFreeMemoryIfPossible, MMgc::_MemoryStatus to=kFreeMemoryIfPossible)  Line 3894                C++ 
                Flash.ocx!MMgc::GCHeap::SendFreeMemorySignal(unsigned int minimumBlocksToFree=164865)  Line 2520 + 0x13 bytes     C++ 
                Flash.ocx!MMgc::GCHeap::Alloc(unsigned int size=164865, unsigned int flags=9)  Line 332            C++ 
(omitted below…)

Comment 1

[Trevor Baker]

need patch

Comment 2

[Brent Getlin]

Attachment: patch

Comment 3

[Tommy Reilly]

I think this has been pushed already, need to double check, patch was confusing because it appears to add back some code I added/then removed.

Comment 4

[Tommy Reilly]

ignore that comment, wrong bug

Comment 5

[Tommy Reilly]

tamarin-redux: changeset:   4344:97de1a6a6381
tamarin-redux-argo: changeset:   3954:97de1a6a6381

Comment 6

[Dan Schaffer]

is there a testcase we can use for marking this bug verified?

Comment 7

[Lars T Hansen]

Dan Schaffer: if there's a test it's going to be very white box.  I'm not sure it's worth the bother.

Comment 8

[Dan Schaffer]

marking as verified.