[PATCH v5 0/9] MR2691: mshtml: More ref leak fixes.
This should finally fix the ref leaks that keep all gecko nsDocuments alive even on the most basic docs. -- v5: 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. https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/nsio.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c index d60b2f0fa81..ca4542c6166 100644 --- a/dlls/mshtml/nsio.c +++ b/dlls/mshtml/nsio.c @@ -3876,16 +3876,6 @@ static nsresult NSAPI nsIOServiceHook_NewURI(nsIIOServiceHook *iface, const nsAC if(!strncmp(spec, "wine:", 5)) spec += 5; - if(aBaseURI) { - nsres = nsIURI_QueryInterface(aBaseURI, &IID_nsWineURI, (void**)&base_wine_uri); - if(NS_SUCCEEDED(nsres)) { - if(!ensure_uri(base_wine_uri)) - return NS_ERROR_UNEXPECTED; - }else { - WARN("Could not get base nsWineURI: %08lx\n", nsres); - } - } - if(aOriginCharset && *aOriginCharset && _strnicmp(aOriginCharset, "utf", 3)) { BSTR charset; int len; @@ -3903,8 +3893,21 @@ static nsresult NSAPI nsIOServiceHook_NewURI(nsIIOServiceHook *iface, const nsAC MultiByteToWideChar(cp, 0, spec, -1, new_spec, ARRAY_SIZE(new_spec)); + if(aBaseURI) { + nsres = nsIURI_QueryInterface(aBaseURI, &IID_nsWineURI, (void**)&base_wine_uri); + if(NS_SUCCEEDED(nsres)) { + if(!ensure_uri(base_wine_uri)) { + nsIFileURL_Release(&base_wine_uri->nsIFileURL_iface); + return NS_ERROR_UNEXPECTED; + } + }else { + WARN("Could not get base nsWineURI: %08lx\n", nsres); + } + } + 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 +3919,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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)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(a)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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)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(a)gmail.com> --- dlls/mshtml/binding.h | 2 +- dlls/mshtml/nsio.c | 125 +++++++++++++++++++++++++++++++++--------- 2 files changed, 101 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 ca4542c6166..efe1f0389f7 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,86 @@ 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 *owner = This->owner; + This->owner = NULL; + nsISupports_Release(owner); + } + if(This->post_data_stream) { + nsIInputStream *post_data_stream = This->post_data_stream; + This->post_data_stream = NULL; + nsIInputStream_Release(post_data_stream); + } + if(This->load_group) { + nsILoadGroup *load_group = This->load_group; + This->load_group = NULL; + nsILoadGroup_Release(load_group); + } + if(This->notif_callback) { + nsIInterfaceRequestor *notif_callback = This->notif_callback; + This->notif_callback = NULL; + nsIInterfaceRequestor_Release(notif_callback); + } + if(This->original_uri) { + nsIURI *original_uri = This->original_uri; + This->original_uri = NULL; + nsIURI_Release(original_uri); + } + if(This->referrer) { + nsIURI *referrer = This->referrer; + This->referrer = NULL; + nsIURI_Release(referrer); + } + + 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 +3585,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; @@ -3990,6 +4058,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; @@ -4009,6 +4082,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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/nsio.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c index efe1f0389f7..6bd9a408037 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(This->load_info, "load_info", cb); if(This->load_group) note_cc_edge((nsISupports*)This->load_group, "load_group", cb); if(This->notif_callback) @@ -2291,6 +2293,11 @@ static nsresult NSAPI nsChannel_unlink(void *p) This->post_data_stream = NULL; nsIInputStream_Release(post_data_stream); } + if(This->load_info) { + nsISupports *load_info = This->load_info; + This->load_info = NULL; + nsISupports_Release(load_info); + } if(This->load_group) { nsILoadGroup *load_group = This->load_group; This->load_group = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Avoids having to look it up again later during collection. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2691
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)