This should finally fix the ref leaks that keep all gecko nsDocuments alive even on the most basic docs.
-- v3: mshtml/tests: Fix element leak in elem_fire_event. mshtml: Pass actual node_ccp to ccref_decr for nodes. mshtml: Fix nsChannel's load_info leak. mshtml: Support cycle collection for nsChannel. mshtml: Store minimum compat mode required for events in the ctor table. mshtml: Fix URI leak in NewURI on failure. mshtml: Fix nsIFile dir leak in init_xpcom. mshtml: Fix factory leak in init_nsio. mshtml: Do not release the principal returned by GetPrincipal.
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsembed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c index 3e315bc7211..16659055295 100644 --- a/dlls/mshtml/nsembed.c +++ b/dlls/mshtml/nsembed.c @@ -2440,6 +2440,7 @@ nsIXMLHttpRequest *create_nsxhr(nsIDOMWindow *nswindow) nsres = nsIGlobalObject_QueryInterface(nsglo, &IID_nsIScriptObjectPrincipal, (void**)&sop); assert(nsres == NS_OK);
+ /* The returned principal is *not* AddRef'd */ nspri = nsIScriptObjectPrincipal_GetPrincipal(sop); nsIScriptObjectPrincipal_Release(sop);
@@ -2451,7 +2452,6 @@ nsIXMLHttpRequest *create_nsxhr(nsIDOMWindow *nswindow) if(NS_FAILED(nsres)) nsIXMLHttpRequest_Release(nsxhr); } - nsISupports_Release(nspri); nsIGlobalObject_Release(nsglo); if(NS_FAILED(nsres)) { ERR("nsIXMLHttpRequest_Init failed: %08lx\n", nsres);
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c index d29bb43544e..d60b2f0fa81 100644 --- a/dlls/mshtml/nsio.c +++ b/dlls/mshtml/nsio.c @@ -4000,9 +4000,9 @@ void init_nsio(nsIComponentManager *component_manager) }
nsres = nsIFactory_CreateInstance(old_factory, NULL, &IID_nsIIOService, (void**)&nsio); + nsIFactory_Release(old_factory); if(NS_FAILED(nsres)) { ERR("Couldn not create nsIOService instance %08lx\n", nsres); - nsIFactory_Release(old_factory); return; }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsembed.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c index 16659055295..9952a553afd 100644 --- a/dlls/mshtml/nsembed.c +++ b/dlls/mshtml/nsembed.c @@ -561,6 +561,7 @@ static BOOL init_xpcom(const PRUnichar *gre_path) }
nsres = NS_InitXPCOM2(&pServMgr, gre_dir, (nsIDirectoryServiceProvider*)&nsDirectoryServiceProvider2); + nsIFile_Release(gre_dir); if(NS_FAILED(nsres)) { ERR("NS_InitXPCOM2 failed: %08lx\n", nsres); FreeLibrary(xul_handle);
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsio.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c index d60b2f0fa81..5bd07334566 100644 --- a/dlls/mshtml/nsio.c +++ b/dlls/mshtml/nsio.c @@ -3892,8 +3892,11 @@ static nsresult NSAPI nsIOServiceHook_NewURI(nsIIOServiceHook *iface, const nsAC
len = MultiByteToWideChar(CP_UTF8, 0, aOriginCharset, -1, NULL, 0); charset = SysAllocStringLen(NULL, len-1); - if(!charset) + if(!charset) { + if(base_wine_uri) + nsIFileURL_Release(&base_wine_uri->nsIFileURL_iface); return NS_ERROR_OUT_OF_MEMORY; + } MultiByteToWideChar(CP_UTF8, 0, aOriginCharset, -1, charset, len);
cp = cp_from_charset_string(charset); @@ -3905,6 +3908,7 @@ static nsresult NSAPI nsIOServiceHook_NewURI(nsIIOServiceHook *iface, const nsAC
if(base_wine_uri) { hres = combine_url(base_wine_uri->uri, new_spec, &urlmon_uri); + nsIFileURL_Release(&base_wine_uri->nsIFileURL_iface); }else { hres = create_uri(new_spec, 0, &urlmon_uri); if(FAILED(hres)) @@ -3916,8 +3920,6 @@ static nsresult NSAPI nsIOServiceHook_NewURI(nsIIOServiceHook *iface, const nsAC
nsres = create_nsuri(urlmon_uri, &wine_uri); IUri_Release(urlmon_uri); - if(base_wine_uri) - nsIFileURL_Release(&base_wine_uri->nsIFileURL_iface); if(NS_FAILED(nsres)) return nsres;
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Since such event types don't exist as separate event types in older modes, this prevents confusing leaks without bloating the constructors with more boilerplate.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmlevent.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index dd0bfe8db02..ccea4e16567 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -3122,9 +3122,6 @@ static DOMEvent *progress_event_ctor(void *iface, nsIDOMEvent *nsevent, eventid_ { DOMProgressEvent *progress_event;
- if(compat_mode < COMPAT_MODE_IE10) - return event_ctor(sizeof(DOMEvent), &DOMEvent_dispex, NULL, NULL, nsevent, event_id, compat_mode); - if(!(progress_event = event_ctor(sizeof(DOMProgressEvent), &DOMProgressEvent_dispex, DOMProgressEvent_query_interface, DOMProgressEvent_destroy, nsevent, event_id, compat_mode))) return NULL; @@ -3154,6 +3151,7 @@ static DOMEvent *storage_event_ctor(void *iface, nsIDOMEvent *nsevent, eventid_t static const struct { REFIID iid; DOMEvent *(*ctor)(void *iface, nsIDOMEvent *nsevent, eventid_t, compat_mode_t); + compat_mode_t min_compat_mode; } event_types_ctor_table[] = { [EVENT_TYPE_EVENT] = { NULL, generic_event_ctor }, [EVENT_TYPE_UIEVENT] = { &IID_nsIDOMUIEvent, ui_event_ctor }, @@ -3164,7 +3162,7 @@ static const struct { [EVENT_TYPE_DRAG] = { NULL, generic_event_ctor }, [EVENT_TYPE_PAGETRANSITION] = { NULL, page_transition_event_ctor }, [EVENT_TYPE_CUSTOM] = { &IID_nsIDOMCustomEvent, custom_event_ctor }, - [EVENT_TYPE_PROGRESS] = { &IID_nsIDOMProgressEvent, progress_event_ctor }, + [EVENT_TYPE_PROGRESS] = { &IID_nsIDOMProgressEvent, progress_event_ctor, COMPAT_MODE_IE10 }, [EVENT_TYPE_MESSAGE] = { NULL, message_event_ctor }, [EVENT_TYPE_STORAGE] = { NULL, storage_event_ctor }, }; @@ -3175,6 +3173,9 @@ static DOMEvent *alloc_event(nsIDOMEvent *nsevent, compat_mode_t compat_mode, ev void *iface = NULL; DOMEvent *event;
+ if(compat_mode < event_types_ctor_table[event_type].min_compat_mode) + event_type = EVENT_TYPE_EVENT; + if(event_types_ctor_table[event_type].iid) nsIDOMEvent_QueryInterface(nsevent, event_types_ctor_table[event_type].iid, &iface);
From: Gabriel Ivăncescu gabrielopcode@gmail.com
This is traversed from the Gecko document, and should be part of the graph (it refers to gecko objects that participate in it, and which are part of a cycle).
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/binding.h | 2 +- dlls/mshtml/nsio.c | 119 +++++++++++++++++++++++++++++++++--------- 2 files changed, 95 insertions(+), 26 deletions(-)
diff --git a/dlls/mshtml/binding.h b/dlls/mshtml/binding.h index 948bfb1c02b..07a8a999ec3 100644 --- a/dlls/mshtml/binding.h +++ b/dlls/mshtml/binding.h @@ -37,7 +37,7 @@ typedef struct { nsIHttpChannelInternal nsIHttpChannelInternal_iface; nsICacheInfoChannel nsICacheInfoChannel_iface;
- LONG ref; + nsCycleCollectingAutoRefCnt ccref;
nsWineURI *uri; nsIInputStream *post_data_stream; diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c index 5bd07334566..e54bb3bc91e 100644 --- a/dlls/mshtml/nsio.c +++ b/dlls/mshtml/nsio.c @@ -45,6 +45,7 @@ static const IID NS_IOSERVICE_CID = static const IID IID_nsWineURI = {0x5088272e, 0x900b, 0x11da, {0xc6,0x87, 0x00,0x0f,0xea,0x57,0xf2,0x1a}};
+static ExternalCycleCollectionParticipant nschannel_ccp; static nsIIOService *nsio = NULL;
static const char *request_method_strings[] = {"GET", "PUT", "POST"}; @@ -540,6 +541,14 @@ static nsresult NSAPI nsChannel_QueryInterface(nsIHttpChannel *iface, nsIIDRef r }else if(IsEqualGUID(&IID_nsICacheInfoChannel, riid)) { TRACE("(%p)->(IID_nsICacheInfoChannel %p)\n", This, result); *result = is_http_channel(This) ? &This->nsICacheInfoChannel_iface : NULL; + }else if(IsEqualGUID(&IID_nsXPCOMCycleCollectionParticipant, riid)) { + TRACE("(%p)->(IID_nsXPCOMCycleCollectionParticipant %p)\n", This, result); + *result = &nschannel_ccp; + return S_OK; + }else if(IsEqualGUID(&IID_nsCycleCollectionISupports, riid)) { + TRACE("(%p)->(IID_nsCycleCollectionISupports %p)\n", This, result); + *result = &This->nsIHttpChannel_iface; + return S_OK; }else { TRACE("(%p)->(%s %p)\n", This, debugstr_guid(riid), result); *result = NULL; @@ -556,7 +565,7 @@ static nsresult NSAPI nsChannel_QueryInterface(nsIHttpChannel *iface, nsIIDRef r static nsrefcnt NSAPI nsChannel_AddRef(nsIHttpChannel *iface) { nsChannel *This = impl_from_nsIHttpChannel(iface); - nsrefcnt ref = InterlockedIncrement(&This->ref); + nsrefcnt ref = ccref_incr(&This->ccref, (nsISupports*)&This->nsIHttpChannel_iface);
TRACE("(%p) ref=%ld\n", This, ref);
@@ -566,30 +575,9 @@ static nsrefcnt NSAPI nsChannel_AddRef(nsIHttpChannel *iface) static nsrefcnt NSAPI nsChannel_Release(nsIHttpChannel *iface) { nsChannel *This = impl_from_nsIHttpChannel(iface); - LONG ref = InterlockedDecrement(&This->ref); + nsrefcnt ref = ccref_decr(&This->ccref, (nsISupports*)&This->nsIHttpChannel_iface, &nschannel_ccp);
- if(!ref) { - nsIFileURL_Release(&This->uri->nsIFileURL_iface); - if(This->owner) - nsISupports_Release(This->owner); - if(This->post_data_stream) - nsIInputStream_Release(This->post_data_stream); - if(This->load_group) - nsILoadGroup_Release(This->load_group); - if(This->notif_callback) - nsIInterfaceRequestor_Release(This->notif_callback); - if(This->original_uri) - nsIURI_Release(This->original_uri); - if(This->referrer) - nsIURI_Release(This->referrer); - - free_http_headers(&This->response_headers); - free_http_headers(&This->request_headers); - - free(This->content_type); - free(This->charset); - free(This); - } + TRACE("(%p) ref=%ld\n", This, ref);
return ref; } @@ -2263,6 +2251,80 @@ static const nsICacheInfoChannelVtbl nsCacheInfoChannelVtbl = { nsCacheInfoChannel_SetAllowStaleCacheContent };
+static nsresult NSAPI nsChannel_traverse(void *ccp, void *p, nsCycleCollectionTraversalCallback *cb) +{ + nsChannel *This = impl_from_nsIHttpChannel(p); + + TRACE("%p\n", This); + + describe_cc_node(&This->ccref, "nsChannel", cb); + + if(This->owner) + note_cc_edge(This->owner, "owner", cb); + if(This->post_data_stream) + note_cc_edge((nsISupports*)This->post_data_stream, "post_data_stream", cb); + if(This->load_group) + note_cc_edge((nsISupports*)This->load_group, "load_group", cb); + if(This->notif_callback) + note_cc_edge((nsISupports*)This->notif_callback, "notif_callback", cb); + if(This->original_uri) + note_cc_edge((nsISupports*)This->original_uri, "original_uri", cb); + if(This->referrer) + note_cc_edge((nsISupports*)This->referrer, "referrer", cb); + + return NS_OK; +} + +static nsresult NSAPI nsChannel_unlink(void *p) +{ + nsChannel *This = impl_from_nsIHttpChannel(p); + + TRACE("%p\n", This); + + if(This->owner) { + nsISupports_Release(This->owner); + This->owner = NULL; + } + if(This->post_data_stream) { + nsIInputStream_Release(This->post_data_stream); + This->post_data_stream = NULL; + } + if(This->load_group) { + nsILoadGroup_Release(This->load_group); + This->load_group = NULL; + } + if(This->notif_callback) { + nsIInterfaceRequestor_Release(This->notif_callback); + This->notif_callback = NULL; + } + if(This->original_uri) { + nsIURI_Release(This->original_uri); + This->original_uri = NULL; + } + if(This->referrer) { + nsIURI_Release(This->referrer); + This->referrer = NULL; + } + + return NS_OK; +} + +static void NSAPI nsChannel_delete_cycle_collectable(void *p) +{ + nsChannel *This = impl_from_nsIHttpChannel(p); + nsChannel_unlink(p); + + TRACE("(%p)\n", This); + + nsIFileURL_Release(&This->uri->nsIFileURL_iface); + free_http_headers(&This->response_headers); + free_http_headers(&This->request_headers); + + free(This->content_type); + free(This->charset); + free(This); +} + static void invalidate_uri(nsWineURI *This) { if(This->uri) { @@ -3517,10 +3579,10 @@ static nsresult create_nschannel(nsWineURI *uri, nsChannel **ret) channel->nsIUploadChannel_iface.lpVtbl = &nsUploadChannelVtbl; channel->nsIHttpChannelInternal_iface.lpVtbl = &nsHttpChannelInternalVtbl; channel->nsICacheInfoChannel_iface.lpVtbl = &nsCacheInfoChannelVtbl; - channel->ref = 1; channel->request_method = METHOD_GET; list_init(&channel->response_headers); list_init(&channel->request_headers); + ccref_init(&channel->ccref, 1);
nsIFileURL_AddRef(&uri->nsIFileURL_iface); channel->uri = uri; @@ -3991,6 +4053,11 @@ static nsIIOServiceHook nsIOServiceHook = { &nsIOServiceHookVtbl };
void init_nsio(nsIComponentManager *component_manager) { + static const CCObjCallback nschannel_ccp_callback = { + nsChannel_traverse, + nsChannel_unlink, + nsChannel_delete_cycle_collectable + }; nsIFactory *old_factory = NULL; nsresult nsres;
@@ -4010,6 +4077,8 @@ void init_nsio(nsIComponentManager *component_manager)
nsres = nsIIOService_SetHook(nsio, &nsIOServiceHook); assert(nsres == NS_OK); + + ccp_init(&nschannel_ccp, &nschannel_ccp_callback); }
void release_nsio(void)
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsio.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c index e54bb3bc91e..8178b87b4ca 100644 --- a/dlls/mshtml/nsio.c +++ b/dlls/mshtml/nsio.c @@ -2263,6 +2263,8 @@ static nsresult NSAPI nsChannel_traverse(void *ccp, void *p, nsCycleCollectionTr note_cc_edge(This->owner, "owner", cb); if(This->post_data_stream) note_cc_edge((nsISupports*)This->post_data_stream, "post_data_stream", cb); + if(This->load_info) + note_cc_edge((nsISupports*)This->load_info, "load_info", cb); if(This->load_group) note_cc_edge((nsISupports*)This->load_group, "load_group", cb); if(This->notif_callback) @@ -2289,6 +2291,10 @@ static nsresult NSAPI nsChannel_unlink(void *p) nsIInputStream_Release(This->post_data_stream); This->post_data_stream = NULL; } + if(This->load_info) { + nsISupports_Release(This->load_info); + This->load_info = NULL; + } if(This->load_group) { nsILoadGroup_Release(This->load_group); This->load_group = NULL;
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Avoids having to look it up again later during collection.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
Not sure why this was commented out, perhaps old wine-gecko version that didn't support it properly? --- dlls/mshtml/htmlnode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/mshtml/htmlnode.c b/dlls/mshtml/htmlnode.c index 5d70b09e234..02f3e2929b2 100644 --- a/dlls/mshtml/htmlnode.c +++ b/dlls/mshtml/htmlnode.c @@ -500,7 +500,7 @@ static ULONG WINAPI HTMLDOMNode_AddRef(IHTMLDOMNode *iface) static ULONG WINAPI HTMLDOMNode_Release(IHTMLDOMNode *iface) { HTMLDOMNode *This = impl_from_IHTMLDOMNode(iface); - LONG ref = ccref_decr(&This->ccref, (nsISupports*)&This->IHTMLDOMNode_iface, /*&node_ccp*/ NULL); + LONG ref = ccref_decr(&This->ccref, (nsISupports*)&This->IHTMLDOMNode_iface, &node_ccp);
TRACE("(%p) ref=%ld\n", This, ref);
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/tests/events.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index ee15ada69ef..7f7515bbcef 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -383,6 +383,7 @@ static void _elem_fire_event(unsigned line, IUnknown *unk, const WCHAR *event, V str = SysAllocString(event); in_fire_event++; hres = IHTMLElement3_fireEvent(elem3, str, evobj, &b); + IHTMLElement3_Release(elem3); in_fire_event--; SysFreeString(str); ok_(__FILE__,line)(hres == S_OK, "fireEvent failed: %08lx\n", hres);
Well unfortunately I wasn't able to find a solution for that CC ref problem yet, so I dropped those patches for now. Guess I'll have to use them locally when analyzing leaks…
I also used minimum compat mode in the event constructor table, I think it's the most clean this way, IMO, even tidying up the constructor slightly. Those event types don't exist in older modes anyway.
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)