Bug 36077

Summary: v8 DOMWrapperWorld needs virtual dtor
Product: WebKit Reporter: James Robinson <jamesr>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

James Robinson
Reported 2010-03-12 16:47:58 PST
v8 DOMWrapperWorld needs virtual dtor
Attachments
Patch (1.33 KB, patch)
2010-03-12 16:49 PST, James Robinson 		no flags
DetailsFormatted DiffDiff
Patch (1.34 KB, patch)
2010-03-12 16:56 PST, James Robinson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
James Robinson
Comment 1 2010-03-12 16:49:04 PST
Created attachment 50641 [details] Patch
Adam Barth
Comment 2 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.
James Robinson
Comment 3 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.
James Robinson
Comment 4 2010-03-12 16:56:19 PST
Created attachment 50642 [details] Patch
Adam Barth
Comment 5 2010-03-12 16:57:10 PST
Comment on attachment 50642 [details] Patch ok
James Robinson
Comment 6 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>
James Robinson
Comment 7 2010-03-12 17:03:07 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.