Bug 36077 - v8 DOMWrapperWorld needs virtual dtor
Summary: v8 DOMWrapperWorld needs virtual dtor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-12 16:47 PST by James Robinson
Modified: 2010-03-12 17:03 PST (History)
0 users

See Also:

Attachments
Patch (1.33 KB, patch)
2010-03-12 16:49 PST, James Robinson 		no flags Details | Formatted Diff | Diff
Patch (1.34 KB, patch)
2010-03-12 16:56 PST, James Robinson 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 James Robinson 2010-03-12 16:47:58 PST 
v8 DOMWrapperWorld needs virtual dtor
Comment 1 James Robinson 2010-03-12 16:49:04 PST 
Created attachment 50641 [details]
Patch
Comment 2 Adam Barth 2010-03-12 16:52:54 PST 
Comment on attachment 50641 [details]
Patch

Nice find!

+ RefCounted<DOMWrapperWorld

should be

+ RefCounted<DOMWrapperWorld>

Also, we usually wrap ChangeLogs at 80 chars.
Comment 3 James Robinson 2010-03-12 16:54:53 PST 
Without this patch the following happens:

V8Proxy::resetIsolatedWorlds() is calling the destructor of each V8IsolatedContext, which each have a  RefPtr<IsolatedWorld> member variable.  Since IsolatedWorld inherits from DOMWrapperWorld, which is declared as RefCounted<DOMWrapperWorld>, the deref() call is doing delete static_cast<DOMWrapperWorld*>(m_ptr).  Since DOMWrapperWorld has no members and does not have a virtual destructor, it just does a free().  This means the IsolatedWorld's member variable, a DOMDataStoreHandle, is never destroyed and so the associated DOMDataStore is leaked and stays in the DOMDataStoreList.
Comment 4 James Robinson 2010-03-12 16:56:19 PST 
Created attachment 50642 [details]
Patch
Comment 5 Adam Barth 2010-03-12 16:57:10 PST 
Comment on attachment 50642 [details]
Patch

ok
Comment 6 James Robinson 2010-03-12 17:03:03 PST 
Comment on attachment 50642 [details]
Patch

Clearing flags on attachment: 50642

Committed r55945: <http://trac.webkit.org/changeset/55945>
Comment 7 James Robinson 2010-03-12 17:03:07 PST 
All reviewed patches have been landed.  Closing bug.