Bug 129393 – ASSERTION FAILED: m_numBreakpoints >= numBreakpoints when deleting breakpoints

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

ASSERTION FAILED: m_numBreakpoints >= numBreakpoints
/Volumes/Data/ws5/OpenSource/Source/JavaScriptCore/bytecode/CodeBlock.h(886) : void JSC::CodeBlock::removeBreakpoint(unsigned int)
1   0x1007368f0 WTFCrash
2   0x10012021e JSC::CodeBlock::removeBreakpoint(unsigned int)
3   0x10011dd8a JSC::Debugger::toggleBreakpoint(JSC::CodeBlock*, JSC::Breakpoint&, JSC::Debugger::BreakpointState)
4   0x10012d8e7 JSC::Debugger::ToggleBreakpointFunctor::operator()(JSC::CodeBlock*)
5   0x10012d836 void JSC::CodeBlockSet::iterate<JSC::Debugger::ToggleBreakpointFunctor>(JSC::Debugger::ToggleBreakpointFunctor&)
6   0x1001203ff void JSC::Heap::forEachCodeBlock<JSC::Debugger::ToggleBreakpointFunctor>(JSC::Debugger::ToggleBreakpointFunctor&)
7   0x10011ddea JSC::Debugger::toggleBreakpoint(JSC::Breakpoint&, JSC::Debugger::BreakpointState)
8   0x10011e6dc JSC::Debugger::removeBreakpoint(unsigned long)
9   0x100697be6 Inspector::ScriptDebugServer::removeBreakpoint(unsigned long)
10  0x10039a462 Inspector::InspectorDebuggerAgent::removeBreakpoint(WTF::String*, WTF::String const&)
11  0x10039a577 non-virtual thunk to Inspector::InspectorDebuggerAgent::removeBreakpoint(WTF::String*, WTF::String const&)
12  0x1003aaf3f Inspector::InspectorDebuggerBackendDispatcher::removeBreakpoint(long, Inspector::InspectorObject const&)
13  0x1003a9be6 Inspector::InspectorDebuggerBackendDispatcher::dispatch(long, WTF::String const&, WTF::PassRefPtr<Inspector::InspectorObject>)
14  0x1003876bc Inspector::InspectorBackendDispatcher::dispatch(WTF::String const&)
15  0x100486191 Inspector::JSGlobalObjectInspectorController::dispatchMessageFromFrontend(WTF::String const&)
16  0x10047c3bb JSC::JSGlobalObjectDebuggable::dispatchMessageFromRemoteFrontend(WTF::String const&)
...

[Radar WebKit Bug Importer]

<rdar://problem/16177455>

[Mark Lam]

The issue manifests because the debugger will iterate all CodeBlocks in the heap when setting / clearing breakpoints, but it is possible for a CodeBlock to have been instantiate but is not yet registered with the debugger.  Hence, any pending breakpoints that would land in that CodeBlock is as yet unset.  As such, an attempt to remove a "would be" breakpoint in that CodeBlock will fail that assertion.

The fix is have the debugger only operate on CodeBlocks that are registered with it.

[Mark Lam]

Created attachment 225327 [details]
the patch.

[Geoffrey Garen]

I think we need a full backtrace here to understand what's going on.

You though the problem was that the compilation thread was still running, but had not yet installed this CodeBlock. If the backtrace verifies that claim, then we have a problem: it's not clear that it's OK for the the profiler to jettison a CodeBlock that has not yet been installed, and is currently compiling. If that's the case, then I think the only solution that will solve both cases is to wait for all compilation to complete before attaching either the debugger or profiler, or modifying breakpoints.

[Mark Lam]

Here are the full backtraces of all threads when the issue manifests.  It doesn't look like compilation is in progress.

(lldb) bt all
  thread #1: tid = 0x6dddd8, 0x00007fff93d7e746 libsystem_kernel.dylib`__psynch_mutexwait + 10, queue = 'com.apple.main-thread'
    frame #0: 0x00007fff93d7e746 libsystem_kernel.dylib`__psynch_mutexwait + 10
    frame #1: 0x00007fff8bf9e779 libsystem_pthread.dylib`_pthread_mutex_lock + 372
    frame #2: 0x00007fff958c57a9 libc++.1.dylib`std::__1::mutex::lock() + 9
    frame #3: 0x000000010049056d JavaScriptCore`JSC::JSLock::lock(this=0x0000000101b09740, lockCount=1) + 157 at JSLock.cpp:120
    frame #4: 0x000000010049012f JavaScriptCore`JSC::JSLock::lock(this=0x0000000101b09740) + 31 at JSLock.cpp:108
    frame #5: 0x0000000100490025 JavaScriptCore`JSC::JSLockHolder::init(this=0x00007fff5fbfce48) + 37 at JSLock.cpp:70
    frame #6: 0x0000000100490098 JavaScriptCore`JSC::JSLockHolder::JSLockHolder(this=0x00007fff5fbfce48, vm=0x0000000104815600) + 56 at JSLock.cpp:59
    frame #7: 0x000000010049004d JavaScriptCore`JSC::JSLockHolder::JSLockHolder(this=0x00007fff5fbfce48, vm=0x0000000104815600) + 29 at JSLock.cpp:60
    frame #8: 0x000000010038337d JavaScriptCore`JSC::APIEntryShim::APIEntryShim(this=0x00007fff5fbfce38, exec=0x00000001063cf9b0, registerThread=true) + 109 at APIShims.h:61
    frame #9: 0x00000001003832dc JavaScriptCore`JSC::APIEntryShim::APIEntryShim(this=0x00007fff5fbfce38, exec=0x00000001063cf9b0, registerThread=true) + 44 at APIShims.h:63
    frame #10: 0x00000001004a9d12 JavaScriptCore`JSObjectCallAsFunction(ctx=0x00000001063cf9b0, object=0x00000001067ddd30, thisObject=0x0000000000000000, argumentCount=2, arguments=0x00007fff5fbfced0, exception=0x0000000000000000) + 66 at JSObjectRef.cpp:522
    frame #11: 0x0000000100004f05 JSPong`-[PongAI nextMove](self=0x0000000101a52b90, _cmd=0x000000010000582a) + 213 at PongAI.m:130
    frame #12: 0x0000000100003cb6 JSPong`-[PongController update](self=0x0000000101e07890, _cmd=0x00007fff9803339e) + 1446 at PongController.m:187
    frame #13: 0x00007fff944e60f4 Foundation`__NSFireTimer + 96
    frame #14: 0x00007fff90bf9564 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
    frame #15: 0x00007fff90bf909f CoreFoundation`__CFRunLoopDoTimer + 1151
    frame #16: 0x00007fff90c6a5aa CoreFoundation`__CFRunLoopDoTimers + 298
    frame #17: 0x00007fff90bb48e5 CoreFoundation`__CFRunLoopRun + 1525
    frame #18: 0x00007fff90bb40b5 CoreFoundation`CFRunLoopRunSpecific + 309
    frame #19: 0x00007fff8fbbea0d HIToolbox`RunCurrentEventLoopInMode + 226
    frame #20: 0x00007fff8fbbe7b7 HIToolbox`ReceiveNextEventCommon + 479
    frame #21: 0x00007fff8fbbe5bc HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 65
    frame #22: 0x00007fff975fd3de AppKit`_DPSNextEvent + 1434
    frame #23: 0x00007fff975fca2b AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 122
    frame #24: 0x00007fff975f0b2c AppKit`-[NSApplication run] + 553
    frame #25: 0x00007fff975db913 AppKit`NSApplicationMain + 940
    frame #26: 0x0000000100001522 JSPong`main(argc=1, argv=0x00007fff5fbffae0) + 34 at main.m:53

  thread #3: tid = 0x6dde07, 0x00007fff93d7f662 libsystem_kernel.dylib`kevent64 + 10, queue = 'com.apple.libdispatch-manager'
    frame #0: 0x00007fff93d7f662 libsystem_kernel.dylib`kevent64 + 10
    frame #1: 0x00007fff8d38043d libdispatch.dylib`_dispatch_mgr_invoke + 239
    frame #2: 0x00007fff8d380152 libdispatch.dylib`_dispatch_mgr_thread + 52

  thread #4: tid = 0x6dde08, 0x00007fff93d7ee6a libsystem_kernel.dylib`__workq_kernreturn + 10
    frame #0: 0x00007fff93d7ee6a libsystem_kernel.dylib`__workq_kernreturn + 10
    frame #1: 0x00007fff8bf9cf08 libsystem_pthread.dylib`_pthread_wqthread + 330

* thread #5: tid = 0x6dde09, 0x000000010073696a JavaScriptCore`WTFCrash + 42 at Assertions.cpp:333, queue = 'com.apple.JavaScriptCore.remote-inspector-xpc-connection', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
  * frame #0: 0x000000010073696a JavaScriptCore`WTFCrash + 42 at Assertions.cpp:333
    frame #1: 0x000000010012021a JavaScriptCore`JSC::CodeBlock::removeBreakpoint(this=0x0000000101914610, numBreakpoints=1) + 74 at CodeBlock.h:888
    frame #2: 0x000000010011ddda JavaScriptCore`JSC::Debugger::toggleBreakpoint(this=0x0000000101c0d960, codeBlock=0x0000000101914610, breakpoint=0x0000000101b1eeb0, enabledOrNot=BreakpointDisabled) + 378 at Debugger.cpp:281
    frame #3: 0x000000010012d95e JavaScriptCore`JSC::Debugger::ToggleBreakpointFunctor::operator(this=0x00000001063804b0, codeBlock=0x0000000101914610)(JSC::CodeBlock*) + 110 at Debugger.cpp:305
    frame #4: 0x000000010012d896 JavaScriptCore`void JSC::CodeBlockSet::iterate<JSC::Debugger::ToggleBreakpointFunctor>(this=0x000000010481f940, functor=0x00000001063804b0) + 118 at CodeBlockSet.h:82
    frame #5: 0x00000001001203ff JavaScriptCore`void JSC::Heap::forEachCodeBlock<JSC::Debugger::ToggleBreakpointFunctor>(this=0x0000000104815618, functor=0x00000001063804b0) + 47 at Heap.h:442
    frame #6: 0x000000010011de3a JavaScriptCore`JSC::Debugger::toggleBreakpoint(this=0x0000000101c0d960, breakpoint=0x0000000101b1eeb0, enabledOrNot=BreakpointDisabled) + 90 at Debugger.cpp:320
    frame #7: 0x000000010011e6ed JavaScriptCore`JSC::Debugger::removeBreakpoint(this=0x0000000101c0d960, id=1) + 621 at Debugger.cpp:393
    frame #8: 0x0000000100697c56 JavaScriptCore`Inspector::ScriptDebugServer::removeBreakpoint(this=0x0000000101c0d960, id=1) + 278 at ScriptDebugServer.cpp:86
    frame #9: 0x000000010039a4e2 JavaScriptCore`Inspector::InspectorDebuggerAgent::removeBreakpoint(this=0x0000000101c0d870, =0x00000001063808d0, breakpointIdentifier=0x0000000106380900) + 370 at InspectorDebuggerAgent.cpp:350
    frame #10: 0x000000010039a5f7 JavaScriptCore`_ZThn24_N9Inspector22InspectorDebuggerAgent16removeBreakpointEPN3WTF6StringERKS2_(this=0x0000000101c0d888, =0x00000001063808d0, breakpointIdentifier=0x0000000106380900) + 55 at InspectorDebuggerAgent.cpp:352
    frame #11: 0x00000001003aafbf JavaScriptCore`Inspector::InspectorDebuggerBackendDispatcher::removeBreakpoint(this=0x0000000101c03540, callId=15, message=0x0000000101c0efd0) + 511 at InspectorJSBackendDispatchers.cpp:333
    frame #12: 0x00000001003a9c66 JavaScriptCore`Inspector::InspectorDebuggerBackendDispatcher::dispatch(this=0x0000000101c03540, callId=15, method=0x0000000106380ba0, message=0x0000000106380b98) + 902 at InspectorJSBackendDispatchers.cpp:213
    frame #13: 0x000000010038773c JavaScriptCore`Inspector::InspectorBackendDispatcher::dispatch(this=0x0000000101c0ed60, message=0x0000000106380d90) + 1596 at InspectorBackendDispatcher.cpp:131
    frame #14: 0x0000000100486201 JavaScriptCore`Inspector::JSGlobalObjectInspectorController::dispatchMessageFromFrontend(this=0x0000000101908cd0, message=0x0000000106380d90) + 81 at JSGlobalObjectInspectorController.cpp:101
    frame #15: 0x000000010047c42b JavaScriptCore`JSC::JSGlobalObjectDebuggable::dispatchMessageFromRemoteFrontend(this=0x0000000101a53c70, message=0x0000000106380d90) + 75 at JSGlobalObjectDebuggable.cpp:70
    frame #16: 0x0000000100686987 JavaScriptCore`___ZN9Inspector35RemoteInspectorDebuggableConnection20sendMessageToBackendEP8NSString_block_invoke(.block_descriptor=<unavailable>) + 183 at RemoteInspectorDebuggableConnection.mm:149
    frame #17: 0x00007fff8d3811d7 libdispatch.dylib`_dispatch_call_block_and_release + 12
    frame #18: 0x00007fff8d37e2ad libdispatch.dylib`_dispatch_client_callout + 8
    frame #19: 0x00007fff8d38068f libdispatch.dylib`_dispatch_queue_drain + 451
    frame #20: 0x00007fff8d3819dd libdispatch.dylib`_dispatch_queue_invoke + 110
    frame #21: 0x00007fff8d37ffa3 libdispatch.dylib`_dispatch_root_queue_drain + 75
    frame #22: 0x00007fff8d381193 libdispatch.dylib`_dispatch_worker_thread2 + 40
    frame #23: 0x00007fff8bf9cef8 libsystem_pthread.dylib`_pthread_wqthread + 314

  thread #7: tid = 0x6dde19, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JavaScriptCore::BlockFree'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x00007fff95891d43 libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x00000001000499fe JavaScriptCore`JSC::BlockAllocator::blockFreeingThreadMain(this=0x0000000104815678) + 734 at BlockAllocator.cpp:144
    frame #4: 0x0000000100049715 JavaScriptCore`JSC::BlockAllocator::blockFreeingThreadStartFunc(blockAllocator=0x0000000104815678) + 21 at BlockAllocator.cpp:120
    frame #5: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000106007f80) + 144 at Threading.cpp:69
    frame #6: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x000000010600afd0) + 296 at ThreadingPthreads.cpp:170
    frame #7: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #8: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #8: tid = 0x6dde1a, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JavaScriptCore::Marking'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x00007fff95891d43 libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100352201 JavaScriptCore`void std::__1::condition_variable::wait<JSC::GCThread::waitForNextPhase(this=0x000000010481f728, __lk=0x000000010a987c78, __pred=<anonymous class> at 0x000000010a987c18)::$_1>(std::__1::unique_lock<std::__1::mutex>&, JSC::GCThread::waitForNextPhase()::$_1) + 65 at __mutex_base:376
    frame #4: 0x00000001003520f6 JavaScriptCore`JSC::GCThread::waitForNextPhase(this=0x0000000101e083c0) + 230 at GCThread.cpp:80
    frame #5: 0x00000001003522aa JavaScriptCore`JSC::GCThread::gcThreadMain(this=0x0000000101e083c0) + 154 at GCThread.cpp:98
    frame #6: 0x00000001003523dd JavaScriptCore`JSC::GCThread::gcThreadStartFunc(data=0x0000000101e083c0) + 29 at GCThread.cpp:134
    frame #7: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000101e0b210) + 144 at Threading.cpp:69
    frame #8: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x0000000101e083e0) + 296 at ThreadingPthreads.cpp:170
    frame #9: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #10: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #9: tid = 0x6dde1b, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JavaScriptCore::Marking'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x00007fff95891d43 libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100352201 JavaScriptCore`void std::__1::condition_variable::wait<JSC::GCThread::waitForNextPhase(this=0x000000010481f728, __lk=0x000000010aa0ac78, __pred=<anonymous class> at 0x000000010aa0ac18)::$_1>(std::__1::unique_lock<std::__1::mutex>&, JSC::GCThread::waitForNextPhase()::$_1) + 65 at __mutex_base:376
    frame #4: 0x00000001003520f6 JavaScriptCore`JSC::GCThread::waitForNextPhase(this=0x0000000101e0af50) + 230 at GCThread.cpp:80
    frame #5: 0x00000001003522aa JavaScriptCore`JSC::GCThread::gcThreadMain(this=0x0000000101e0af50) + 154 at GCThread.cpp:98
    frame #6: 0x00000001003523dd JavaScriptCore`JSC::GCThread::gcThreadStartFunc(data=0x0000000101e0af50) + 29 at GCThread.cpp:134
    frame #7: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000101e076f0) + 144 at Threading.cpp:69
    frame #8: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x0000000101e07750) + 296 at ThreadingPthreads.cpp:170
    frame #9: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #10: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #10: tid = 0x6dde1c, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JavaScriptCore::Marking'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x00007fff95891d43 libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100352201 JavaScriptCore`void std::__1::condition_variable::wait<JSC::GCThread::waitForNextPhase(this=0x000000010481f728, __lk=0x000000010aa8dc78, __pred=<anonymous class> at 0x000000010aa8dc18)::$_1>(std::__1::unique_lock<std::__1::mutex>&, JSC::GCThread::waitForNextPhase()::$_1) + 65 at __mutex_base:376
    frame #4: 0x00000001003520f6 JavaScriptCore`JSC::GCThread::waitForNextPhase(this=0x0000000101e09660) + 230 at GCThread.cpp:80
    frame #5: 0x00000001003522aa JavaScriptCore`JSC::GCThread::gcThreadMain(this=0x0000000101e09660) + 154 at GCThread.cpp:98
    frame #6: 0x00000001003523dd JavaScriptCore`JSC::GCThread::gcThreadStartFunc(data=0x0000000101e09660) + 29 at GCThread.cpp:134
    frame #7: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000101e07240) + 144 at Threading.cpp:69
    frame #8: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x0000000101e0b510) + 296 at ThreadingPthreads.cpp:170
    frame #9: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #10: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #11: tid = 0x6dde1d, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JavaScriptCore::Marking'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x00007fff95891d43 libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100352201 JavaScriptCore`void std::__1::condition_variable::wait<JSC::GCThread::waitForNextPhase(this=0x000000010481f728, __lk=0x000000010ab10c78, __pred=<anonymous class> at 0x000000010ab10c18)::$_1>(std::__1::unique_lock<std::__1::mutex>&, JSC::GCThread::waitForNextPhase()::$_1) + 65 at __mutex_base:376
    frame #4: 0x00000001003520f6 JavaScriptCore`JSC::GCThread::waitForNextPhase(this=0x0000000101e09680) + 230 at GCThread.cpp:80
    frame #5: 0x00000001003522aa JavaScriptCore`JSC::GCThread::gcThreadMain(this=0x0000000101e09680) + 154 at GCThread.cpp:98
    frame #6: 0x00000001003523dd JavaScriptCore`JSC::GCThread::gcThreadStartFunc(data=0x0000000101e09680) + 29 at GCThread.cpp:134
    frame #7: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000101e0b840) + 144 at Threading.cpp:69
    frame #8: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x0000000101e09db0) + 296 at ThreadingPthreads.cpp:170
    frame #9: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #10: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #12: tid = 0x6dde1e, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JavaScriptCore::Marking'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x00007fff95891d43 libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100352201 JavaScriptCore`void std::__1::condition_variable::wait<JSC::GCThread::waitForNextPhase(this=0x000000010481f728, __lk=0x000000010ab93c78, __pred=<anonymous class> at 0x000000010ab93c18)::$_1>(std::__1::unique_lock<std::__1::mutex>&, JSC::GCThread::waitForNextPhase()::$_1) + 65 at __mutex_base:376
    frame #4: 0x00000001003520f6 JavaScriptCore`JSC::GCThread::waitForNextPhase(this=0x0000000101e09000) + 230 at GCThread.cpp:80
    frame #5: 0x00000001003522aa JavaScriptCore`JSC::GCThread::gcThreadMain(this=0x0000000101e09000) + 154 at GCThread.cpp:98
    frame #6: 0x00000001003523dd JavaScriptCore`JSC::GCThread::gcThreadStartFunc(data=0x0000000101e09000) + 29 at GCThread.cpp:134
    frame #7: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000101e09020) + 144 at Threading.cpp:69
    frame #8: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x0000000101e0b080) + 296 at ThreadingPthreads.cpp:170
    frame #9: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #10: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #13: tid = 0x6dde1f, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JavaScriptCore::Marking'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x00007fff95891d43 libc++.1.dylib`std::__1::condition_variable::wait(std::__1::unique_lock<std::__1::mutex>&) + 47
    frame #3: 0x0000000100352201 JavaScriptCore`void std::__1::condition_variable::wait<JSC::GCThread::waitForNextPhase(this=0x000000010481f728, __lk=0x000000010ac16c78, __pred=<anonymous class> at 0x000000010ac16c18)::$_1>(std::__1::unique_lock<std::__1::mutex>&, JSC::GCThread::waitForNextPhase()::$_1) + 65 at __mutex_base:376
    frame #4: 0x00000001003520f6 JavaScriptCore`JSC::GCThread::waitForNextPhase(this=0x0000000101e091d0) + 230 at GCThread.cpp:80
    frame #5: 0x00000001003522aa JavaScriptCore`JSC::GCThread::gcThreadMain(this=0x0000000101e091d0) + 154 at GCThread.cpp:98
    frame #6: 0x00000001003523dd JavaScriptCore`JSC::GCThread::gcThreadStartFunc(data=0x0000000101e091d0) + 29 at GCThread.cpp:134
    frame #7: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000101e091f0) + 144 at Threading.cpp:69
    frame #8: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x0000000101e04180) + 296 at ThreadingPthreads.cpp:170
    frame #9: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #10: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #14: tid = 0x6dde20, 0x00007fff93d7aa1a libsystem_kernel.dylib`mach_msg_trap + 10
    frame #0: 0x00007fff93d7aa1a libsystem_kernel.dylib`mach_msg_trap + 10
    frame #1: 0x00007fff93d79d18 libsystem_kernel.dylib`mach_msg + 64
    frame #2: 0x00007fff90bb5155 CoreFoundation`__CFRunLoopServiceMachPort + 181
    frame #3: 0x00007fff90bb4779 CoreFoundation`__CFRunLoopRun + 1161
    frame #4: 0x00007fff90bb40b5 CoreFoundation`CFRunLoopRunSpecific + 309
    frame #5: 0x00007fff9779d16e AppKit`_NSEventThread + 144
    frame #6: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #7: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137

  thread #15: tid = 0x6dde67, 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10, name = 'JSC Compilation Thread'
    frame #0: 0x00007fff93d7e716 libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff8bf9dc3b libsystem_pthread.dylib`_pthread_cond_wait + 727
    frame #2: 0x0000000100787b40 JavaScriptCore`WTF::ThreadCondition::wait(this=0x00000001019119d8, mutex=0x0000000101911998) + 48 at ThreadingPthreads.cpp:342
    frame #3: 0x000000010032fa5e JavaScriptCore`JSC::DFG::Worklist::runThread(this=0x0000000101911870, data=0x000000010190b520) + 190 at DFGWorklist.cpp:273
    frame #4: 0x000000010032e804 JavaScriptCore`JSC::DFG::Worklist::threadFunction(argument=0x000000010190b520) + 36 at DFGWorklist.cpp:315
    frame #5: 0x00000001007861d0 JavaScriptCore`WTF::threadEntryPoint(contextData=0x0000000101911a50) + 144 at Threading.cpp:69
    frame #6: 0x0000000100786e58 JavaScriptCore`WTF::wtfThreadEntryPoint(param=0x000000010190c880) + 296 at ThreadingPthreads.cpp:170
    frame #7: 0x00007fff8bf9b899 libsystem_pthread.dylib`_pthread_body + 138
    frame #8: 0x00007fff8bf9b72a libsystem_pthread.dylib`_pthread_start + 137
(lldb)

[Mark Lam]

For the next step, I'll do some digging into the life cycle of that CodeBlock, and try to find out why its m_jitCode is still null.  All I know so far is that the CodeBlock has been added to the heap's CodeBlockSet but has not yet been installed with its executable.

[Geoffrey Garen]

Comment on attachment 225327 [details]
the patch.

Looks like the initial explanation for this patch turned out to be false, so I'll mark this r-.

[Mark Lam]

* thread #1: tid = 0x71811f, 0x00000001000b0eeb JavaScriptCore`JSC::CodeBlock::CodeBlock(this=0x0000000101c0ff90, =CopyParsedBlock, other=0x0000000101926730) + 1915 at CodeBlock.cpp:1477, queue = 'com.apple.main-thread', stop reason = breakpoint 3.1
  * frame #0: 0x00000001000b0eeb JavaScriptCore`JSC::CodeBlock::CodeBlock(this=0x0000000101c0ff90, =CopyParsedBlock, other=0x0000000101926730) + 1915 at CodeBlock.cpp:1477
    frame #1: 0x0000000100347a21 JavaScriptCore`JSC::FunctionCodeBlock::FunctionCodeBlock(this=0x0000000101c0ff90, =CopyParsedBlock, other=0x0000000101926730) + 49 at CodeBlock.h:1191
    frame #2: 0x0000000100345543 JavaScriptCore`JSC::FunctionCodeBlock::FunctionCodeBlock(this=0x0000000101c0ff90, =CopyParsedBlock, other=0x0000000101926730) + 35 at CodeBlock.h:1192
    frame #3: 0x0000000100342af1 JavaScriptCore`JSC::ScriptExecutable::newReplacementCodeBlockFor(this=0x000000010b41fd70, kind=CodeForCall) + 833 at Executable.cpp:275
    frame #4: 0x00000001000b77b3 JavaScriptCore`JSC::CodeBlock::newReplacement(this=0x0000000101926730) + 67 at CodeBlock.cpp:2662
    frame #5: 0x00000001003f7120 JavaScriptCore`operationOptimize(exec=0x00007fff5fbfca00, bytecodeIndex=0) + 1968 at JITOperations.cpp:1184
    frame #6: 0x000051bd10c02912

Here's the stack trace that added the offending CodeBlock.

[Mark Lam]

(In reply to comment #8)
> * thread #1: tid = 0x71811f, 0x00000001000b0eeb JavaScriptCore`JSC::CodeBlock::CodeBlock(this=0x0000000101c0ff90, =CopyParsedBlock, other=0x0000000101926730) + 1915 at CodeBlock.cpp:1477, queue = 'com.apple.main-thread', stop reason = breakpoint 3.1
...
>     frame #5: 0x00000001003f7120 JavaScriptCore`operationOptimize(exec=0x00007fff5fbfca00, bytecodeIndex=0) + 1968 at JITOperations.cpp:1184
>     frame #6: 0x000051bd10c02912
> 
> Here's the stack trace that added the offending CodeBlock.

And the CompilationResult that was returned by DFG::compile() after instantiating that CodeBlock is CompilationInvalidated (and I know this because I added some printfs to log the CodeBlock and the CompilationResult).

However, this code is before my recent fix for <https://webkit.org/b/129355>.  That fix may change this behavior.  Will svn up and re-test.

[Geoffrey Garen]

So, it seems there were three issues here:

(1) The debugger and profiler used to force CompilationInvalidated all the time.

(2) If CompilationInvalidated happens, we'll end up with a CodeBlock that never registers with the debugger.

(3) During async compilation, there is at least one CodeBlock that has not yet registered with the debugger or profiler, and will never register with the profiler.

[Mark Lam]

(In reply to comment #10)
> So, it seems there were three issues here:
> 
> (1) The debugger and profiler used to force CompilationInvalidated all the time.

With <https://webkit.org/b/129355> which fixes this issue, I can no longer reproduce the assertion failure in this bug.  Will think about the other 2 issues.

[Mark Lam]

Created attachment 225616 [details]
patch 2: eagerly reaps zombie CodeBlocks, and waits for compilations to complete before proceeding with debugger/profiler work

[Mark Lam]

After more analysis, here's an update on the issue and the fix:

The issue manifests because the debugger will iterate all CodeBlocks in the heap when setting / clearing breakpoints, but it is possible for a CodeBlock to have been instantiate but is not yet registered with the debugger.  This can happen because of the following:

1. DFG worklist compilation is still in progress, and the target codeBlock is not ready for installation in its executable yet.

2. DFG compilation failed and we have a codeBlock that will never be installed in its executable, and the codeBlock has not been cleaned up by the GC yet.

The code for installing the codeBlock in its executable is the same code that registers it with the debugger.  Hence, these codeBlocks are not registered with the debugger, and any pending breakpoints that would map to that CodeBlock is as yet unset or will never be set.  As such, an attempt to remove a breakpoint in that CodeBlock will fail that assertion.

To fix this, we do the following:

1. We'll eagerly clean up any zombie CodeBlocks due to failed DFG / FTL compilation.  This is achieved by providing a DeferredCompilationCallback::compilationDidComplete() that does this clean up, and have all sub classes call it at the end of their compilationDidComplete() methods.

2. Before the debugger or profiler iterates CodeBlocks in the heap, they will wait for all compilations to complete before proceeding.  This ensures that:
    1. any zombie CodeBlocks would have been cleaned up, and won't be seen by the debugger or profiler.
    2. all CodeBlocks that the debugger and profiler needs to operate on will be "ready" for whatever needs to be done to them e.g. jettison'ing of DFG codeBlocks.

[Geoffrey Garen]

Comment on attachment 225616 [details]
patch 2: eagerly reaps zombie CodeBlocks, and waits for compilations to complete before proceeding with debugger/profiler work

View in context: https://bugs.webkit.org/attachment.cgi?id=225616&action=review

r=me

> Source/JavaScriptCore/bytecode/DeferredCompilationCallback.cpp:41
> +void DeferredCompilationCallback::compilationDidComplete(CodeBlock* codeBlock, CompilationResult result)
> +{
> +    ASSERT(result == CompilationFailed || result == CompilationInvalidated || result == CompilationSuccessful);
> +    if (result != CompilationSuccessful)
> +        codeBlock->heap()->removeCodeBlock(codeBlock);
> +}

I would use a switch statement here. That way, the compiler will check your assertion at compile time.

> Source/JavaScriptCore/heap/CodeBlockSet.h:70
> +    // Remove a CodeBlock. This is only called when compilation is not successful
> +    // in generating the jitCode for the CodeBlock.
> +    void remove(CodeBlock*);

It's a layering violation to make promises about who will call this function and / or when. Let's remove this comment.

> Source/JavaScriptCore/runtime/VM.h:502
> -        void prepareToDiscardCode();
> +        void prepareToDiscardCode() { waitForCompilationsToComplete(); }
> +        void waitForCompilationsToComplete();

Instead of having two names for the same thing, let's pick one and have all callers use it.

[Mark Lam]

Thanks.  The feedback has been applied.  Landed in r165005: <http://trac.webkit.org/r165005>.