Bug 78645 - RootObject::finalize can cause a crash in object->invalidate()
Summary: RootObject::finalize can cause a crash in object->invalidate()
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
Depends on:
Reported: 2012-02-14 15:38 PST by Mark Hahnenberg
Modified: 2012-02-15 14:02 PST (History)
2 users (show)

See Also:

Patch (1.39 KB, patch)
2012-02-14 15:43 PST, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (1.36 KB, patch)
2012-02-14 17:08 PST, Mark Hahnenberg
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2012-02-14 15:38:57 PST
If we finalize weak handles and call RootObject::finalize(), it calls invalidate() on the object that it's finalizing and then removes it from its map of RuntimeObjects. However, invalidate() derefs that RuntimeObject which can call its destructor. In turn, it will deref its member RefPtr to the RootObject. If that RootObject's ref count then hits 0, its destructor will be called, which will then call invalidate() on all its objects in its map. This causes invalidate() to be called on that RuntimeObject twice, which causes a crash.

Removing the object from the map first and then calling invalidate() on it should alleviate the crash because invalidate() can't be called a second time if the RootObject's destructor gets called since it will no longer be in the map.
Comment 1 Mark Hahnenberg 2012-02-14 15:39:21 PST
Comment 2 Mark Hahnenberg 2012-02-14 15:43:45 PST
Created attachment 127067 [details]
Comment 3 Geoffrey Garen 2012-02-14 16:45:08 PST
Comment on attachment 127067 [details]

This looks good, but I think an even better fix would be "RefPtr<T> protect(this);"
Comment 4 Mark Hahnenberg 2012-02-14 17:08:18 PST
Created attachment 127083 [details]
Comment 5 Geoffrey Garen 2012-02-15 09:29:27 PST
Comment on attachment 127083 [details]

Comment 6 Mark Hahnenberg 2012-02-15 14:02:17 PST
Committed r107837: <http://trac.webkit.org/changeset/107837>