Bug 172432 - Web Inspector: Using "break on all exceptions" when throwing stack overflow hangs inspector
Summary: Web Inspector: Using "break on all exceptions" when throwing stack overflow h...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-05-21 16:06 PDT by Saam Barati
Modified: 2017-06-21 14:32 PDT (History)
8 users (show)

See Also:


Attachments
[PATCH] Proposed Fix v4 (5.64 KB, patch)
2017-06-20 17:25 PDT, Joseph Pecoraro
saam: review+
joepeck: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix - With Tests (11.82 KB, patch)
2017-06-21 13:53 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix - With Tests (11.68 KB, patch)
2017-06-21 13:57 PDT, 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 Saam Barati 2017-05-21 16:06:17 PDT
e.g, if I enable "Break on all exceptions" and run this program, web inspector hangs for me. My guess is it's trying to do stuff above the JS stack at the time of stack overflow, which if true, is definitely not allowed.

```
let arg = 20
function f() {
    f(arg);
}

function recurse() {
    try {
        f();
    } catch(e) {
    }
    setTimeout(recurse, 0);
}
recurse();
```
Comment 1 Matt Baker 2017-05-22 11:30:18 PDT
<rdar://problem/29870873>
Comment 2 Joseph Pecoraro 2017-06-20 17:25:35 PDT
Created attachment 313459 [details]
[PATCH] Proposed Fix v4
Comment 3 Saam Barati 2017-06-20 17:45:25 PDT
Comment on attachment 313459 [details]
[PATCH] Proposed Fix v4

r=me
Can you add a test that ensures we don't catch it in inspector?
Comment 4 Joseph Pecoraro 2017-06-20 18:41:56 PDT
Comment on attachment 313459 [details]
[PATCH] Proposed Fix v4

Oops, yes I totally meant to add a test.
Comment 5 Joseph Pecoraro 2017-06-21 13:53:27 PDT
Created attachment 313546 [details]
[PATCH] Proposed Fix - With Tests
Comment 6 Joseph Pecoraro 2017-06-21 13:54:57 PDT
Comment on attachment 313546 [details]
[PATCH] Proposed Fix - With Tests

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

> LayoutTests/inspector/debugger/no-pause-stack-overflow-exception.html:21
> +    WebInspector.debuggerManager.assertionsBreakpoint.disabled = false;

I can drop the assertions breakpoint. I really just need the all exceptions one.
Comment 7 Joseph Pecoraro 2017-06-21 13:57:24 PDT
Created attachment 313547 [details]
[PATCH] Proposed Fix - With Tests
Comment 8 WebKit Commit Bot 2017-06-21 14:32:47 PDT
Comment on attachment 313547 [details]
[PATCH] Proposed Fix - With Tests

Clearing flags on attachment: 313547

Committed r218652: <http://trac.webkit.org/changeset/218652>
Comment 9 WebKit Commit Bot 2017-06-21 14:32:49 PDT
All reviewed patches have been landed.  Closing bug.