Bug 69453 - save resource zeroing stacks in ScriptElement for debugging
Summary: save resource zeroing stacks in ScriptElement for debugging
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Peters
URL:
Keywords:
Depends on: 69472
Blocks:
  Show dependency tree / graph
 
Reported: 2011-10-05 12:26 PDT by Gavin Peters
Modified: 2011-10-06 09:40 PDT (History)
5 users (show)

See Also:

Attachments
Patch (3.13 KB, patch)
2011-10-05 12:29 PDT, Gavin Peters 		no flags Details | Formatted Diff | Diff
Patch (3.13 KB, patch)
2011-10-05 12:35 PDT, Gavin Peters 		no flags Details | Formatted Diff | Diff
Patch (3.27 KB, patch)
2011-10-05 12:55 PDT, Gavin Peters 		no flags Details | Formatted Diff | Diff
Patch (3.25 KB, patch)
2011-10-06 06:50 PDT, Gavin Peters 		japhet: review+
japhet: commit-queue+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Peters 2011-10-05 12:26:33 PDT 
save resource zeroing stacks in ScriptElement for debugging
Comment 1 Gavin Peters 2011-10-05 12:29:21 PDT 
Created attachment 109837 [details]
Patch
Comment 2 Gavin Peters 2011-10-05 12:30:33 PDT 
add more data for crashes in ScriptElement::notifyFinished double calls.
Comment 3 Adam Barth 2011-10-05 12:33:00 PDT 
Comment on attachment 109837 [details]
Patch

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

> Source/WebCore/dom/ScriptElement.h:120
> +      MaxBacktraceSize = 32

bad indent
Comment 4 Gavin Peters 2011-10-05 12:35:18 PDT 
Created attachment 109838 [details]
Patch
Comment 5 Gavin Peters 2011-10-05 12:36:06 PDT 
Comment on attachment 109838 [details]
Patch

Fixed indent.
Comment 6 Nate Chapin 2011-10-05 12:50:37 PDT 
Comment on attachment 109838 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        two notifyFinished() events in a ScriptElement, which can crash chrome (see chrome bug
> +        75604).  This patch will save a stack in the ScriptElement when this happens, so that if

Please provide a link to the chromium bug.

> Source/WebCore/dom/ScriptElement.h:122
> +
> +    // We grab a backtrace when we zero m_cachedScript, so that at later crashes
> +    // we'll have a debuggable stack.
> +    enum {
> +        MaxBacktraceSize = 32
> +    };
> +    int m_backtraceSize;

Does this value ever change?  Can we just make it a static const in ScriptElement.cpp?
Comment 7 Gavin Peters 2011-10-05 12:55:04 PDT 
Created attachment 109842 [details]
Patch
Comment 8 Gavin Peters 2011-10-05 12:57:37 PDT 
Comment on attachment 109838 [details]
Patch

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

>> Source/WebCore/ChangeLog:8
>> +        75604).  This patch will save a stack in the ScriptElement when this happens, so that if
> 
> Please provide a link to the chromium bug.

Done.

>> Source/WebCore/dom/ScriptElement.h:122
>> +    int m_backtraceSize;
> 
> Does this value ever change?  Can we just make it a static const in ScriptElement.cpp?

It does change, it contains the actual size of the stack in m_backtrace, it could be anything from 0 to 32.
Comment 9 Gavin Peters 2011-10-05 12:58:07 PDT 
Comment on attachment 109842 [details]
Patch

Now with links to the chromium bug database.  Boy, isn't the chromium bug database really unbearably slow?
Comment 10 WebKit Review Bot 2011-10-05 15:02:42 PDT 
Comment on attachment 109842 [details]
Patch

Clearing flags on attachment: 109842

Committed r96756: <http://trac.webkit.org/changeset/96756>
Comment 11 WebKit Review Bot 2011-10-05 15:02:46 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 12 Adam Roben (:aroben) 2011-10-05 15:35:56 PDT 
This broke many builds due to WTFGetBacktrace not being exported from JavaScriptCore. Rolling it out in bug 69472.
Comment 13 Gavin Peters 2011-10-06 06:38:13 PDT 
I just added a new patch to bug 69018 which exports these functions, after that this patch should be good to go, possibly after rerunning ews.
Comment 14 Gavin Peters 2011-10-06 06:50:52 PDT 
Created attachment 109957 [details]
Patch
Comment 15 Gavin Peters 2011-10-06 06:54:01 PDT 
Comment on attachment 109957 [details]
Patch

This patch is unchanged from the version that got a rollout; however I've since updated the exports from JavaScriptCore so it should work now.  Let's wait on the windows ews bot in particular, then land it?
Comment 16 Nate Chapin 2011-10-06 09:05:00 PDT 
Comment on attachment 109957 [details]
Patch

Looks like it's passed the bots this time, so r+ again.
Comment 17 Gavin Peters 2011-10-06 09:40:15 PDT 
Committed r96819: <http://trac.webkit.org/changeset/96819>