125876 – DelayedReleaseScope is in the wrong place

RESOLVED FIXED 125876

DelayedReleaseScope is in the wrong place

https://bugs.webkit.org/show_bug.cgi?id=125876

Summary DelayedReleaseScope is in the wrong place

Mark Hahnenberg

Reported 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.

Attachments
Patch (1.51 KB, patch) 2013-12-18 14:51 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Patch (3.89 KB, patch) 2013-12-18 16:37 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Mark Hahnenberg

Comment 1 2013-12-18 14:51:43 PST

Created attachment 219572 [details] Patch

Geoffrey Garen

Comment 2 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.

Mark Hahnenberg

Comment 3 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).

Mark Hahnenberg

Comment 4 2013-12-18 16:37:05 PST

Created attachment 219587 [details] Patch

Geoffrey Garen

Comment 5 2013-12-18 17:04:02 PST

Comment on attachment 219587 [details] Patch r=me

WebKit Commit Bot

Comment 6 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>

WebKit Commit Bot

Comment 7 2013-12-18 20:26:45 PST

All reviewed patches have been landed. Closing bug.

Geoffrey Garen

Comment 8 2013-12-18 21:43:35 PST

Can we API test for this?

Joseph Pecoraro

Comment 9 2013-12-19 00:48:49 PST

Thanks for the quick fix!

Joseph Pecoraro

Comment 10 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....

Mark Hahnenberg

Comment 11 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.

Joseph Pecoraro

Comment 12 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.

Geoffrey Garen

Comment 13 2014-01-23 14:40:41 PST

Yes, I think so.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component JavaScriptCore

Assignee

Mark Hahnenberg

Reported

2013-12-17 15:18 PST

Modified

2014-01-23 14:40 PST History

CC List

3 users Show

URL

Keywords

Depends on

Blocks