Bug 139797 - Web Inspector: CRASH inspector-protocol/debugger/breakpoint-action-detach.html
Summary: Web Inspector: CRASH inspector-protocol/debugger/breakpoint-action-detach.html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-12-18 16:20 PST by Joseph Pecoraro
Modified: 2014-12-19 11:37 PST (History)
10 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (9.01 KB, patch)
2014-12-18 16:25 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.