Bug 107146 - IndexedDB: Prevent crash dereferencing null if script context has stopped
Summary: IndexedDB: Prevent crash dereferencing null if script context has stopped
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: Joshua Bell
URL:
Keywords:
Depends on: 107050
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-17 11:14 PST by Joshua Bell
Modified: 2013-01-18 11:22 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.35 KB, patch)
2013-01-17 11:24 PST, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch for landing (2.35 KB, patch)
2013-01-17 11:43 PST, Joshua Bell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2013-01-17 11:14:49 PST
IndexedDB: Prevent crash dereferencing null if script context has stopped
Comment 1 Joshua Bell 2013-01-17 11:24:23 PST
Created attachment 183227 [details]
Patch
Comment 2 Joshua Bell 2013-01-17 11:28:01 PST
For more context, see:

https://code.google.com/p/chromium/issues/detail?id=168503
https://bugs.webkit.org/show_bug.cgi?id=107050

Since the patch in 107050 didn't prevent the issue it's (probably) not a null context coming in during IDBRequest creation. Therefore the context must be getting cleared later, either by corruption or a call to stop(). Detect the latter and avoid the deference.

If this works we can merge this to earlier branches and track down the root cause.
Comment 3 Tony Chang 2013-01-17 11:36:55 PST
Comment on attachment 183227 [details]
Patch

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

OK

> Source/WebCore/Modules/indexeddb/IDBRequest.cpp:443
> +    // FIXME: This method not be called if stop() was previously called, but

This method *should* not ...
Comment 4 Tony Chang 2013-01-17 11:38:15 PST
Comment on attachment 183227 [details]
Patch

Another tactic is to use CRASH() to assert conditions in releases.  People have done this in the past to track down unknown crashers.
Comment 5 Joshua Bell 2013-01-17 11:43:44 PST
Created attachment 183232 [details]
Patch for landing
Comment 6 Joshua Bell 2013-01-17 11:44:59 PST
(In reply to comment #3)
> > Source/WebCore/Modules/indexeddb/IDBRequest.cpp:443
> > +    // FIXME: This method not be called if stop() was previously called, but
> 
> This method *should* not ...

Fixed.

(In reply to comment #4)
> (From update of attachment 183227 [details])
> Another tactic is to use CRASH() to assert conditions in releases.  People have done this in the past to track down unknown crashers.

Good to know - may end up using that as this investigation continues.
Comment 7 WebKit Review Bot 2013-01-17 12:39:22 PST
Comment on attachment 183232 [details]
Patch for landing

Clearing flags on attachment: 183232

Committed r140027: <http://trac.webkit.org/changeset/140027>
Comment 8 WebKit Review Bot 2013-01-17 12:39:25 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Joshua Bell 2013-01-18 11:22:02 PST
Good news - this appears to have worked.

Now we just need to figure out *why*.