Bug 139797

Summary: Web Inspector: CRASH inspector-protocol/debugger/breakpoint-action-detach.html
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, graouts, joepeck, jonowells, mark.lam, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2014-12-18 16:20:24 PST
* SUMMARY
CRASH running inspector-protocol/debugger/breakpoint-action-detach.html with guard malloc.

* STEPS TO REPRODUCE:
1. shell> run-webkit-tests inspector-protocol/debugger/breakpoint-action-detach.html --repeat-each 2 -v --no-retry -g
  => CRASH

* CRASH
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010310bffc WTF::Vector<Inspector::ScriptBreakpointAction, 0ul, WTF::CrashOnOverflow>::size() const + 12
1   com.apple.JavaScriptCore      	0x000000010347732c Inspector::ScriptDebugServer::handleBreakpointHit(JSC::Breakpoint const&) + 204
2   com.apple.JavaScriptCore      	0x0000000102d18a96 JSC::Debugger::pauseIfNeeded(JSC::ExecState*) + 422
3   com.apple.JavaScriptCore      	0x0000000102d18d6c JSC::Debugger::updateCallFrameAndPauseIfNeeded(JSC::ExecState*) + 60
4   com.apple.JavaScriptCore      	0x0000000102d18f8c JSC::Debugger::returnEvent(JSC::ExecState*) + 76
5   com.apple.JavaScriptCore      	0x0000000103177069 JSC::Interpreter::debug(JSC::ExecState*, JSC::DebugHookID) + 265

* NOTES
- The test itself is closing the page in the 1st breakpoint action, and assumes the 2nd breakpoint action never fires:

    actions: [
        {type: "evaluate", data: "disconnect()"},
        {type: "evaluate", data: "log('FAIL: This action should not be executed.')"}
    ]

- Iterating breakpoint actions we actually use a reference to a list in a hash, unfortunately that hash entry can go away.

This is enough to fix the crash:

     BreakpointIDToActionsMap::iterator it = m_breakpointIDToActions.find(breakpoint.id);
     if (it != m_breakpointIDToActions.end()) {
-        BreakpointActions& actions = it->value;
+        BreakpointActions actions = it->value;
         for (size_t i = 0; i < actions.size(); ++i) {
             if (!evaluateBreakpointAction(actions[i]))
                 return;
         }
     }

but we need to do a bit more to ensure that later breakpoint actions don't still fire if the debugger was detached.
Comment 1 Radar WebKit Bug Importer 2014-12-18 16:21:05 PST
<rdar://problem/19301619>
Comment 2 Joseph Pecoraro 2014-12-18 16:25:19 PST
Created attachment 243531 [details]
[PATCH] Proposed Fix

All breakpoint action tests pass. Also played with breakpoint actions a bit manually to make sure multiple breakpoint actions behaved as expected.
Comment 3 Mark Lam 2014-12-19 10:33:51 PST
Comment on attachment 243531 [details]
[PATCH] Proposed Fix

r=me
Comment 4 WebKit Commit Bot 2014-12-19 11:37:24 PST
Comment on attachment 243531 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 243531

Committed r177585: <http://trac.webkit.org/changeset/177585>
Comment 5 WebKit Commit Bot 2014-12-19 11:37:28 PST
All reviewed patches have been landed.  Closing bug.