Bug 125876

Summary: DelayedReleaseScope is in the wrong place
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, joepeck
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Mark Hahnenberg 2013-12-17 15:18:48 PST 
Since it could be in the middle of lazy sweeping (and therefore allocation), we can't allow other threads to start executing in the current VM. Thus, we need to hold onto the API lock.
Comment 1 Mark Hahnenberg 2013-12-18 14:51:43 PST 
Created attachment 219572 [details]
Patch
Comment 2 Geoffrey Garen 2013-12-18 15:17:51 PST 
Comment on attachment 219572 [details]
Patch

Mark has a better idea.

We think this patch would not have solved reentrancy.
Comment 3 Mark Hahnenberg 2013-12-18 16:33:02 PST 
It needs to be just around the free list sweeping in MarkedAllocator::tryAllocateHelper. This location gives us a good safe point between getting ready to allocate  (i.e. identifying a non-empty free list) and doing the actual allocation (popping the free list).
Comment 4 Mark Hahnenberg 2013-12-18 16:37:05 PST 
Created attachment 219587 [details]
Patch
Comment 5 Geoffrey Garen 2013-12-18 17:04:02 PST 
Comment on attachment 219587 [details]
Patch

r=me
Comment 6 WebKit Commit Bot 2013-12-18 20:26:43 PST 
Comment on attachment 219587 [details]
Patch

Clearing flags on attachment: 219587

Committed r160822: <http://trac.webkit.org/changeset/160822>
Comment 7 WebKit Commit Bot 2013-12-18 20:26:45 PST 
All reviewed patches have been landed.  Closing bug.
Comment 8 Geoffrey Garen 2013-12-18 21:43:35 PST 
Can we API test for this?
Comment 9 Joseph Pecoraro 2013-12-19 00:48:49 PST 
Thanks for the quick fix!
Comment 10 Joseph Pecoraro 2013-12-19 12:20:41 PST 
Hmm, I'm still seeing a related issue:

ASSERTION FAILED: m_operationInProgress == NoOperation
/Volumes/Data/Code/safari/OpenSource/Source/JavaScriptCore/heap/Heap.cpp(759) : void JSC::Heap::collect(JSC::Heap::SweepToggle)
1   0x1006a33d0 WTFCrash
2   0x1003310a6 JSC::Heap::collect(JSC::Heap::SweepToggle)
3   0x100326310 JSC::DefaultGCActivityCallback::doWork()
4   0x100340172 JSC::HeapTimer::timerDidFire(__CFRunLoopTimer*, void*)
5   0x7fff88f5b724 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
6   0x7fff88f5b25f __CFRunLoopDoTimer
7   0x7fff88fcc76a __CFRunLoopDoTimers
8   0x7fff88f16aa5 __CFRunLoopRun
9   0x7fff88f16275 CFRunLoopRunSpecific
10  0x7fff942abf0d RunCurrentEventLoopInMode
11  0x7fff942abcb7 ReceiveNextEventCommon
12  0x7fff942ababc _BlockUntilNextEventMatchingListInModeWithFilter
13  0x7fff8db6f28e _DPSNextEvent
14  0x7fff8db6e8db -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
15  0x7fff8db629cc -[NSApplication run]
16  0x7fff8db4d803 NSApplicationMain
17  0x100002022 main
18  0x7fff89c225fd start
19  0x3


(lldb) thread backtrace all
* thread #1: tid = 0xd83a4a, 0x00000001006a33da JavaScriptCore`WTFCrash + 42 at Assertions.cpp:341, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
    frame #0: 0x00000001006a33da JavaScriptCore`WTFCrash + 42 at Assertions.cpp:341
    frame #1: 0x00000001003310a6 JavaScriptCore`JSC::Heap::collect(this=0x0000000101859218, sweepToggle=DoNotSweep) + 502 at Heap.cpp:759
    frame #2: 0x0000000100326310 JavaScriptCore`JSC::DefaultGCActivityCallback::doWork(this=0x000060800012d200) + 208 at GCActivityCallback.cpp:98
    frame #3: 0x0000000100340172 JavaScriptCore`JSC::HeapTimer::timerDidFire(timer=0x000060800016b880, context=0x00006080000d62d0) + 338 at HeapTimer.cpp:97
    frame #4: 0x00007fff88f5b724 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
    frame #5: 0x00007fff88f5b25f CoreFoundation`__CFRunLoopDoTimer + 1151
    frame #6: 0x00007fff88fcc76a CoreFoundation`__CFRunLoopDoTimers + 298
    frame #7: 0x00007fff88f16aa5 CoreFoundation`__CFRunLoopRun + 1525
    frame #8: 0x00007fff88f16275 CoreFoundation`CFRunLoopRunSpecific + 309
    frame #9: 0x00007fff942abf0d HIToolbox`RunCurrentEventLoopInMode + 226
    frame #10: 0x00007fff942abcb7 HIToolbox`ReceiveNextEventCommon + 479
    frame #11: 0x00007fff942ababc HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 65
    frame #12: 0x00007fff8db6f28e AppKit`_DPSNextEvent + 1434
    frame #13: 0x00007fff8db6e8db AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 122
    frame #14: 0x00007fff8db629cc AppKit`-[NSApplication run] + 553
    frame #15: 0x00007fff8db4d803 AppKit`NSApplicationMain + 940
    frame #16: 0x0000000100002022 JSContextTester`main(argc=3, argv=0x00007fff5fbffb50) + 34 at main.m:13
    frame #17: 0x00007fff89c225fd libdyld.dylib`start + 1

  thread #2: tid = 0xd83a8f, 0x00007fff8ee7ae6a libsystem_kernel.dylib`__workq_kernreturn + 10
    frame #0: 0x00007fff8ee7ae6a libsystem_kernel.dylib`__workq_kernreturn + 10
    frame #1: 0x00007fff92325f08 libsystem_pthread.dylib`_pthread_wqthread + 330
    frame #2: 0x00007fff92328fb9 libsystem_pthread.dylib`start_wqthread + 13

  thread #3: tid = 0xd83a90, 0x00007fff8ee7b662 libsystem_kernel.dylib`kevent64 + 10, queue = 'com.apple.libdispatch-manager
    frame #0: 0x00007fff8ee7b662 libsystem_kernel.dylib`kevent64 + 10
    frame #1: 0x00007fff926e543d libdispatch.dylib`_dispatch_mgr_invoke + 239
    frame #2: 0x00007fff926e5152 libdispatch.dylib`_dispatch_mgr_thread + 52

  thread #4: tid = 0xd83a91, 0x00007fff8ee7a746 libsystem_kernel.dylib`__psynch_mutexwait + 10, queue = 'com.apple.JavaScriptCore.remote-inspector-xpc-connection
    frame #0: 0x00007fff8ee7a746 libsystem_kernel.dylib`__psynch_mutexwait + 10
    frame #1: 0x00007fff92327779 libsystem_pthread.dylib`_pthread_mutex_lock + 372
    frame #2: 0x00000001006f2095 JavaScriptCore`WTF::Mutex::lock(this=0x00006080000d62d8) + 21 at ThreadingPthreads.cpp:343
    frame #3: 0x0000000100400e8f JavaScriptCore`JSC::JSLock::lock(this=0x00006080000d62d0) + 191 at JSLock.cpp:117
    frame #4: 0x0000000100401387 JavaScriptCore`JSC::JSLock::grabAllLocks(this=0x00006080000d62d0, lockCount=2, spinLock=0x00006080000d62d4) + 55 at JSLock.cpp:277
    frame #5: 0x0000000100401706 JavaScriptCore`JSC::JSLock::DropAllLocks::~DropAllLocks(this=0x000000010227f9b0) + 150 at JSLock.cpp:323
    frame #6: 0x0000000100401665 JavaScriptCore`JSC::JSLock::DropAllLocks::~DropAllLocks(this=0x000000010227f9b0) + 21 at JSLock.cpp:316
    frame #7: 0x000000010033d06d JavaScriptCore`JSC::APICallbackShim::~APICallbackShim(this=0x000000010227f9b0) + 61 at APIShims.h:102
    frame #8: 0x000000010033cce5 JavaScriptCore`JSC::APICallbackShim::~APICallbackShim(this=0x000000010227f9b0) + 21 at APIShims.h:100
    frame #9: 0x000000010033cc55 JavaScriptCore`JSC::DelayedReleaseScope::~DelayedReleaseScope(this=0x000000010227fa40) + 149 at DelayedReleaseScope.h:52
    frame #10: 0x00000001003339e5 JavaScriptCore`JSC::DelayedReleaseScope::~DelayedReleaseScope(this=0x000000010227fa40) + 21 at DelayedReleaseScope.h:46
    frame #11: 0x00000001004d8623 JavaScriptCore`JSC::MarkedAllocator::tryAllocateHelper(this=0x000000010185fbe8, bytes=88) + 451 at MarkedAllocator.cpp:65
    frame #12: 0x00000001004d6f82 JavaScriptCore`JSC::MarkedAllocator::tryAllocate(this=0x000000010185fbe8, bytes=88) + 114 at MarkedAllocator.cpp:78
    frame #13: 0x00000001004d69f5 JavaScriptCore`JSC::MarkedAllocator::allocateSlowCase(this=0x000000010185fbe8, bytes=88) + 245 at MarkedAllocator.cpp:96
    frame #14: 0x000000010001931f JavaScriptCore`JSC::MarkedAllocator::allocate(this=0x000000010185fbe8, bytes=88) + 79 at MarkedAllocator.h:91
    frame #15: 0x00000001000191b9 JavaScriptCore`JSC::MarkedSpace::allocateWithoutDestructor(this=0x00000001018594c8, bytes=88) + 41 at MarkedSpace.h:219
    frame #16: 0x00000001000190c6 JavaScriptCore`JSC::Heap::allocateWithoutDestructor(this=0x0000000101859218, bytes=88) + 118 at Heap.h:443
    frame #17: 0x00000001000c5067 JavaScriptCore`void* JSC::allocateCell<JSC::JSActivation>(heap=0x0000000101859218, size=88) + 151 at JSCellInlines.h:97
    frame #18: 0x00000001000c4f75 JavaScriptCore`JSC::JSActivation::create(vm=0x0000000101859200, callFrame=0x000000010ae47e30, registers=0x000000010ae47e30, codeBlock=0x000000010250a610) + 133 at JSActivation.h:57
    frame #19: 0x00000001000add64 JavaScriptCore`JSC::JSActivation::create(vm=0x0000000101859200, callFrame=0x000000010ae47e30, codeBlock=0x000000010250a610) + 68 at JSActivation.h:66
    frame #20: 0x00000001004ad826 JavaScriptCore`llint_slow_path_create_activation(exec=0x000000010ae47e30, pc=0x000000010481b820) + 134 at LLIntSlowPaths.cpp:446
    frame #21: 0x00000001004b8055 JavaScriptCore`llint_op_create_activation + 32

  thread....
Comment 11 Mark Hahnenberg 2013-12-19 13:15:28 PST 
(In reply to comment #10)
> Hmm, I'm still seeing a related issue:
> 
> ASSERTION FAILED: m_operationInProgress == NoOperation
> /Volumes/Data/Code/safari/OpenSource/Source/JavaScriptCore/heap/Heap.cpp(759) : void JSC::Heap::collect(JSC::Heap::SweepToggle)
> 1   0x1006a33d0 WTFCrash
> 2   0x1003310a6 JSC::Heap::collect(JSC::Heap::SweepToggle)
> 3   0x100326310 JSC::DefaultGCActivityCallback::doWork()
> 4   0x100340172 JSC::HeapTimer::timerDidFire(__CFRunLoopTimer*, void*)
> 5   0x7fff88f5b724 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
> 6   0x7fff88f5b25f __CFRunLoopDoTimer
> 7   0x7fff88fcc76a __CFRunLoopDoTimers
> 8   0x7fff88f16aa5 __CFRunLoopRun
> 9   0x7fff88f16275 CFRunLoopRunSpecific
> 10  0x7fff942abf0d RunCurrentEventLoopInMode
> 11  0x7fff942abcb7 ReceiveNextEventCommon
> 12  0x7fff942ababc _BlockUntilNextEventMatchingListInModeWithFilter
> 13  0x7fff8db6f28e _DPSNextEvent
> 14  0x7fff8db6e8db -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
> 15  0x7fff8db629cc -[NSApplication run]
> 16  0x7fff8db4d803 NSApplicationMain
> 17  0x100002022 main
> 18  0x7fff89c225fd start
> 19  0x3
> 
> 
> (lldb) thread backtrace all
> * thread #1: tid = 0xd83a4a, 0x00000001006a33da JavaScriptCore`WTFCrash + 42 at Assertions.cpp:341, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
>     frame #0: 0x00000001006a33da JavaScriptCore`WTFCrash + 42 at Assertions.cpp:341
>     frame #1: 0x00000001003310a6 JavaScriptCore`JSC::Heap::collect(this=0x0000000101859218, sweepToggle=DoNotSweep) + 502 at Heap.cpp:759
>     frame #2: 0x0000000100326310 JavaScriptCore`JSC::DefaultGCActivityCallback::doWork(this=0x000060800012d200) + 208 at GCActivityCallback.cpp:98
>     frame #3: 0x0000000100340172 JavaScriptCore`JSC::HeapTimer::timerDidFire(timer=0x000060800016b880, context=0x00006080000d62d0) + 338 at HeapTimer.cpp:97
>     frame #4: 0x00007fff88f5b724 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
>     frame #5: 0x00007fff88f5b25f CoreFoundation`__CFRunLoopDoTimer + 1151
>     frame #6: 0x00007fff88fcc76a CoreFoundation`__CFRunLoopDoTimers + 298
>     frame #7: 0x00007fff88f16aa5 CoreFoundation`__CFRunLoopRun + 1525
>     frame #8: 0x00007fff88f16275 CoreFoundation`CFRunLoopRunSpecific + 309
>     frame #9: 0x00007fff942abf0d HIToolbox`RunCurrentEventLoopInMode + 226
>     frame #10: 0x00007fff942abcb7 HIToolbox`ReceiveNextEventCommon + 479
>     frame #11: 0x00007fff942ababc HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 65
>     frame #12: 0x00007fff8db6f28e AppKit`_DPSNextEvent + 1434
>     frame #13: 0x00007fff8db6e8db AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 122
>     frame #14: 0x00007fff8db629cc AppKit`-[NSApplication run] + 553
>     frame #15: 0x00007fff8db4d803 AppKit`NSApplicationMain + 940
>     frame #16: 0x0000000100002022 JSContextTester`main(argc=3, argv=0x00007fff5fbffb50) + 34 at main.m:13
>     frame #17: 0x00007fff89c225fd libdyld.dylib`start + 1
> 
>   thread #2: tid = 0xd83a8f, 0x00007fff8ee7ae6a libsystem_kernel.dylib`__workq_kernreturn + 10
>     frame #0: 0x00007fff8ee7ae6a libsystem_kernel.dylib`__workq_kernreturn + 10
>     frame #1: 0x00007fff92325f08 libsystem_pthread.dylib`_pthread_wqthread + 330
>     frame #2: 0x00007fff92328fb9 libsystem_pthread.dylib`start_wqthread + 13
> 
>   thread #3: tid = 0xd83a90, 0x00007fff8ee7b662 libsystem_kernel.dylib`kevent64 + 10, queue = 'com.apple.libdispatch-manager
>     frame #0: 0x00007fff8ee7b662 libsystem_kernel.dylib`kevent64 + 10
>     frame #1: 0x00007fff926e543d libdispatch.dylib`_dispatch_mgr_invoke + 239
>     frame #2: 0x00007fff926e5152 libdispatch.dylib`_dispatch_mgr_thread + 52
> 
>   thread #4: tid = 0xd83a91, 0x00007fff8ee7a746 libsystem_kernel.dylib`__psynch_mutexwait + 10, queue = 'com.apple.JavaScriptCore.remote-inspector-xpc-connection
>     frame #0: 0x00007fff8ee7a746 libsystem_kernel.dylib`__psynch_mutexwait + 10
>     frame #1: 0x00007fff92327779 libsystem_pthread.dylib`_pthread_mutex_lock + 372
>     frame #2: 0x00000001006f2095 JavaScriptCore`WTF::Mutex::lock(this=0x00006080000d62d8) + 21 at ThreadingPthreads.cpp:343
>     frame #3: 0x0000000100400e8f JavaScriptCore`JSC::JSLock::lock(this=0x00006080000d62d0) + 191 at JSLock.cpp:117
>     frame #4: 0x0000000100401387 JavaScriptCore`JSC::JSLock::grabAllLocks(this=0x00006080000d62d0, lockCount=2, spinLock=0x00006080000d62d4) + 55 at JSLock.cpp:277
>     frame #5: 0x0000000100401706 JavaScriptCore`JSC::JSLock::DropAllLocks::~DropAllLocks(this=0x000000010227f9b0) + 150 at JSLock.cpp:323
>     frame #6: 0x0000000100401665 JavaScriptCore`JSC::JSLock::DropAllLocks::~DropAllLocks(this=0x000000010227f9b0) + 21 at JSLock.cpp:316
>     frame #7: 0x000000010033d06d JavaScriptCore`JSC::APICallbackShim::~APICallbackShim(this=0x000000010227f9b0) + 61 at APIShims.h:102
>     frame #8: 0x000000010033cce5 JavaScriptCore`JSC::APICallbackShim::~APICallbackShim(this=0x000000010227f9b0) + 21 at APIShims.h:100
>     frame #9: 0x000000010033cc55 JavaScriptCore`JSC::DelayedReleaseScope::~DelayedReleaseScope(this=0x000000010227fa40) + 149 at DelayedReleaseScope.h:52
>     frame #10: 0x00000001003339e5 JavaScriptCore`JSC::DelayedReleaseScope::~DelayedReleaseScope(this=0x000000010227fa40) + 21 at DelayedReleaseScope.h:46
>     frame #11: 0x00000001004d8623 JavaScriptCore`JSC::MarkedAllocator::tryAllocateHelper(this=0x000000010185fbe8, bytes=88) + 451 at MarkedAllocator.cpp:65
>     frame #12: 0x00000001004d6f82 JavaScriptCore`JSC::MarkedAllocator::tryAllocate(this=0x000000010185fbe8, bytes=88) + 114 at MarkedAllocator.cpp:78
>     frame #13: 0x00000001004d69f5 JavaScriptCore`JSC::MarkedAllocator::allocateSlowCase(this=0x000000010185fbe8, bytes=88) + 245 at MarkedAllocator.cpp:96
>     frame #14: 0x000000010001931f JavaScriptCore`JSC::MarkedAllocator::allocate(this=0x000000010185fbe8, bytes=88) + 79 at MarkedAllocator.h:91
>     frame #15: 0x00000001000191b9 JavaScriptCore`JSC::MarkedSpace::allocateWithoutDestructor(this=0x00000001018594c8, bytes=88) + 41 at MarkedSpace.h:219
>     frame #16: 0x00000001000190c6 JavaScriptCore`JSC::Heap::allocateWithoutDestructor(this=0x0000000101859218, bytes=88) + 118 at Heap.h:443
>     frame #17: 0x00000001000c5067 JavaScriptCore`void* JSC::allocateCell<JSC::JSActivation>(heap=0x0000000101859218, size=88) + 151 at JSCellInlines.h:97
>     frame #18: 0x00000001000c4f75 JavaScriptCore`JSC::JSActivation::create(vm=0x0000000101859200, callFrame=0x000000010ae47e30, registers=0x000000010ae47e30, codeBlock=0x000000010250a610) + 133 at JSActivation.h:57
>     frame #19: 0x00000001000add64 JavaScriptCore`JSC::JSActivation::create(vm=0x0000000101859200, callFrame=0x000000010ae47e30, codeBlock=0x000000010250a610) + 68 at JSActivation.h:66
>     frame #20: 0x00000001004ad826 JavaScriptCore`llint_slow_path_create_activation(exec=0x000000010ae47e30, pc=0x000000010481b820) + 134 at LLIntSlowPaths.cpp:446
>     frame #21: 0x00000001004b8055 JavaScriptCore`llint_op_create_activation + 32
> 
>   thread....

Grr, looks like that ASSERT is now in the wrong place. As a work-around you can comment it out for now.
Comment 12 Joseph Pecoraro 2014-01-23 14:06:54 PST 
> Grr, looks like that ASSERT is now in the wrong place. As a work-around you can comment it out for now.

Should I file a new bug to get this addressed? I still see it happening.
Comment 13 Geoffrey Garen 2014-01-23 14:40:41 PST 
Yes, I think so.