Deadlock during memoryStatusChange notifications

VERIFIED FIXED in flash10.1

Status

Tamarin
Garbage Collection (mmGC)
P2
normal
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: Brent Getlin, Assigned: Brent Getlin)

Tracking

unspecified
flash10.1
x86
Windows Vista
Bug Flags:
flashplayer-qrb +

Details

(Whiteboard: Has patch)

Attachments

(1 attachment)

1.13 KB, patch
Tommy Reilly
: review+
Lars T Hansen
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
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…)

Updated

8 years ago
Component: Virtual Machine → Garbage Collection (mmGC)
Priority: -- → P2
QA Contact: vm → gc
Target Milestone: --- → flash10.1

Comment 1

8 years ago
need patch
(Assignee)

Comment 2

8 years ago
Created attachment 436787 [details] [diff] [review]
patch
Attachment #436787 - Flags: superreview?(lhansen)
Attachment #436787 - Flags: review?(treilly)

Updated

8 years ago
Attachment #436787 - Flags: superreview?(lhansen) → superreview+

Updated

8 years ago
Status: NEW → ASSIGNED
Flags: flashplayer-qrb+
Whiteboard: Has patch

Comment 3

8 years ago
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

8 years ago
ignore that comment, wrong bug

Updated

8 years ago
Attachment #436787 - Flags: review?(treilly) → review+

Comment 5

8 years ago
tamarin-redux: changeset:   4344:97de1a6a6381
tamarin-redux-argo: changeset:   3954:97de1a6a6381
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
QA Contact: gc → dschaffe

Comment 6

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

Comment 7

8 years ago
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

8 years ago
marking as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.