Summary: | REGRESSION: Amazon.com crash (ActivationImp) | ||
---|---|---|---|
Product: | WebKit | Reporter: | Cameron Zwarich (cpst) <zwarich> |
Component: | JavaScriptCore | Assignee: | Geoffrey Garen <ggaren> |
Status: | RESOLVED FIXED | ||
Severity: | Major | CC: | mjs |
Priority: | P1 | Keywords: | InRadar, Regression |
Version: | 528+ (Nightly build) | ||
Hardware: | Mac | ||
OS: | OS X 10.5 | ||
URL: | http://www.amazon.com/ | ||
Attachments: |
Description
Cameron Zwarich (cpst)
2008-01-17 12:22:35 PST
Created attachment 18509 [details]
Stack trace
Strangely enough, I can't actually get it to crash with the GC modified to always collect after an allocation. Perhaps it is a timing issue that the extra garbage collection eliminates.
Here is a stack trace of a typical crash.
Alternate steps for a crash on Amazon (which are more reliable for me): 1. Go to Amazon.com 2. Search for "Hoboken Chicken Emergency" 3. Open the link for the first product result in a background tab. Maciej's instructions reliably reproduce the crash for me, with a similar looking stack trace to what I posted. Still, changing it to garbage collect every allocation makes the crash impossible to reproduce. Instead of crashing, it prints a JavaScript object such as function n2RunEvent(sWhen) { goN2Initializer.run(sWhen); } or [object Object] at the top of the page. Created attachment 18518 [details]
ExecState activity log
Tearing off every ActivationImp as soon as it is created fixes the crash, which suggests that the problem is due to a missed reference to an ActivationImp from an ExecState that should be explicitly mark()'d but isn't. Tearing off the activation puts a pointer into the GC heap in m_activation, which is caught by the conservative collector.
Maciej suggested I log ExecState creations, deletions, and markings so that we can check for anything suspicious. I've attached such a log from a session that crashed.
Created attachment 18520 [details]
execstate activity log annotated with GC START and GC END markers
Created attachment 18521 [details]
perl script to find inconsistencies
Created attachment 18522 [details]
results of perl script (look for the XXX)
With more investigation, I found that some live ExecStates fail to get marked specifically because script execution (via the <script> element) sometimes breaks both the callingExec and savedExec chains. I have a fix but it might be bad for performance - will have to test. Created attachment 18523 [details]
this patch is a .4% SunSpider regression so it can't go in as-is but I think I know how to make it viable
(In reply to comment #1) > <rdar://problem/5691998> Cameron, please add the "InRadar" keyword to a bug if you file them in Radar. Thanks! Created attachment 18549 [details]
.7% speedup - do we have a winner?
Created attachment 18552 [details]
The accompanying WebCore patch
Created attachment 18556 [details]
This patch builds and passes regression tests
Comment on attachment 18556 [details]
This patch builds and passes regression tests
r=me
Thanks for finishing this off. I'd guess we can also reinstate the LocalStorage optimization since all ExecStates will be tracked?
I'm working on landing this now. With a little work, yes, I believe we can re-instate the LocalStorage buffer caching optimization. Committed revision 29710. |