Summary: | IndexedDB: Possible null ScriptExecutionContext passed to callbacks during frame destruction | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joshua Bell <jsbell> | ||||||
Component: | WebCore Misc. | Assignee: | Joshua Bell <jsbell> | ||||||
Status: | RESOLVED INVALID | ||||||||
Severity: | Normal | CC: | alecflett, dgrogan, tony, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 36565, 107141 | ||||||||
Bug Blocks: | 107146 | ||||||||
Attachments: |
|
Description
Joshua Bell
2013-01-16 13:34:32 PST
Created attachment 183029 [details]
Patch
Comment on attachment 183029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183029&action=review > Source/WebCore/ChangeLog:7 > + Temporary code to defend against null contexts. Will either refute a hypothesis, or we'll > + need to make a more systemic fix elsewhere. Either way it will be removed in a few days. How will we decide whether it worked or not? Is this something we're seeing in the wild or something that cluster fuzz caught? I'm a bit nervous about this even if it fixes the crash because it might be hiding some other bug. We need a repro case to be certain. Someone who knows more about frame destruction (abarth?) can probably comment whether this ought to be needed or not. Sorry, should have linked to crbug.com/168503 for context. We've been unable to repro locally, but some users can do so reliably with a simple repro case. We're still working on trying to repro it ourselves using virtual machines, etc. I see, you might want to mention that WorkerScriptController::controllerForContext() does a similar check. Does the change that added the check in WorkerScriptController::controllerForContext() include a test for this case? Maybe that will provide some hints? I would also mention in the changelog that we're unable to repro and we're watching crash reports from users. Comment on attachment 183029 [details]
Patch
Seems OK to try, but please explain more in the ChangeLog.
Thanks, tony@ - great ideas. For reference: https://bugs.webkit.org/show_bug.cgi?id=36565 - doesn't look like new tests were added. (In reply to comment #6) > Thanks, tony@ - great ideas. > > For reference: > > https://bugs.webkit.org/show_bug.cgi?id=36565 - doesn't look like new tests were added. That was a code move. The original patch that added the lines is https://bugs.webkit.org/show_bug.cgi?id=28004 , which also doesn't have a test :( Created attachment 183039 [details]
Patch for landing
Comment on attachment 183039 [details] Patch for landing Clearing flags on attachment: 183039 Committed r139929: <http://trac.webkit.org/changeset/139929> All reviewed patches have been landed. Closing bug. Reports from users that the crash still occurs. After verifying, will likely back this out. Re-opened since this is blocked by bug 107141 Rolled this out. Closing as INVALID since this wasn't the proximal cause of the crashes we're seeing. |