This should finally fix the ref leaks that keep all gecko nsDocuments alive even on the most basic docs.
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 3e315bc7211..17247975106 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 we don't make use of the iface and it's transferred to the constructor, we have to discard it.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmlevent.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index dd0bfe8db02..a38997b284a 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -3122,8 +3122,10 @@ static DOMEvent *progress_event_ctor(void *iface, nsIDOMEvent *nsevent, eventid_ { DOMProgressEvent *progress_event;
- if(compat_mode < COMPAT_MODE_IE10) + if(compat_mode < COMPAT_MODE_IE10) { + nsIDOMProgressEvent_Release(iface); 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)))
From: Gabriel Ivăncescu gabrielopcode@gmail.com
This fixes leaks when some Gecko thread events hold refs to the document or other objects.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsembed.c | 27 +++++++++++++++++++++++++++ dlls/mshtml/nsiface.idl | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+)
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c index 17247975106..62a6ba009dc 100644 --- a/dlls/mshtml/nsembed.c +++ b/dlls/mshtml/nsembed.c @@ -46,6 +46,7 @@ WINE_DECLARE_DEBUG_CHANNEL(gecko); #define NS_PREFERENCES_CONTRACTID "@mozilla.org/preferences;1" #define NS_VARIANT_CONTRACTID "@mozilla.org/variant;1" #define NS_CATEGORYMANAGER_CONTRACTID "@mozilla.org/categorymanager;1" +#define NS_THREADMANAGER_CONTRACTID "@mozilla.org/thread-manager;1" #define NS_XMLHTTPREQUEST_CONTRACTID "@mozilla.org/xmlextras/xmlhttprequest;1"
#define PR_UINT32_MAX 0xffffffff @@ -1103,6 +1104,29 @@ nsresult get_nsinterface(nsISupports *iface, REFIID riid, void **ppv) return nsres; }
+static void flush_thread_nsevents(void) +{ + nsIThreadManager *threadmgr; + nsIThread *thread; + nsresult nsres; + cpp_bool b; + + nsres = nsIServiceManager_GetServiceByContractID(pServMgr, NS_THREADMANAGER_CONTRACTID, &IID_nsIThreadManager, (void**)&threadmgr); + if(NS_FAILED(nsres)) + return; + + nsres = nsIThreadManager_GetCurrentThread(threadmgr, &thread); + nsIThreadManager_Release(threadmgr); + if(NS_FAILED(nsres)) + return; + + do { + nsres = nsIThread_ProcessNextEvent(thread, FALSE, &b); + } while(NS_SUCCEEDED(nsres) && b); + + nsIThread_Release(thread); +} + static HRESULT nsnode_to_nsstring_rec(nsIContentSerializer *serializer, nsIDOMNode *nsnode, nsAString *str) { nsIDOMNodeList *node_list = NULL; @@ -2398,6 +2422,9 @@ void detach_gecko_browser(GeckoBrowser *This)
nsIWebBrowserChrome_Release(&This->nsIWebBrowserChrome_iface);
+ /* Flush gecko thread events that might be holding refs */ + flush_thread_nsevents(); + /* Force cycle collection */ if(window_utils) { nsIDOMWindowUtils_CycleCollect(window_utils, NULL, 0); diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl index 5991e0de45b..2189e65a5d5 100644 --- a/dlls/mshtml/nsiface.idl +++ b/dlls/mshtml/nsiface.idl @@ -270,6 +270,46 @@ interface nsIRunnable : nsISupports nsresult Run(); }
+[ + object, + uuid(88145945-3278-424e-9f37-d874cbdd9f6f), + local +] +interface nsIEventTarget : nsISupports +{ + nsresult IsOnCurrentThread(bool *aResult); + nsresult Dispatch(nsIRunnable *aEvent, uint32_t aFlags); + nsresult DispatchFromScript(nsIRunnable *aEvent, uint32_t aFlags); +} + +[ + object, + uuid(5801d193-29d1-4964-a6b7-70eb697ddf2b), + local +] +interface nsIThread : nsIEventTarget +{ + nsresult GetPRThread(void /*PRThread*/ **aResult); + nsresult Shutdown(); + nsresult HasPendingEvents(bool *aResult); + nsresult ProcessNextEvent(bool aMayWait, bool *aResult); + nsresult AsyncShutdown(); +} + +[ + object, + uuid(1be89eca-e2f7-453b-8d38-c11ba247f6f3), + local +] +interface nsIThreadManager : nsISupports +{ + nsresult NewThread(uint32_t aCreationFlags, uint32_t aStackSize, nsIThread **aResult); + nsresult GetThreadFromPRThread(void /*PRThread*/ *aThread, nsIThread **aResult); + nsresult GetMainThread(nsIThread **aResult); + nsresult GetCurrentThread(nsIThread **aResult); + nsresult GetIsMainThread(bool *aResult); +} + [ object, uuid(d1899240-f9d2-11d2-bdd6-000064657374),
From: Gabriel Ivăncescu gabrielopcode@gmail.com
When forcing cycle collection manually, Gecko treats it as a specific type of collection. This kind of collection, unfortunately, will not always clean everything, unlike when shutting down the CC, which results in leaks when closing a doc obj because when we force it we expect it to behave like a shutdown for the given doc obj.
Enabling graph logging *will* however clean everything (with env vars), to be able to track all the nodes. To get the same behavior, we can supply a dummy logger that fails early when opening the files (after which wine-gecko replaces the logger with NULL, except now it always visits everything, which is what we want), without actually shutting down the CC, as we aren't actually shutting down, so another doc obj might be created later. And there's no XPCOM interface for shutting it down, anyway.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsembed.c | 116 +++++++++++++++++++++++++++++++++++++++- dlls/mshtml/nsiface.idl | 38 ++++++++++++- 2 files changed, 152 insertions(+), 2 deletions(-)
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c index 62a6ba009dc..3ab256e0a0c 100644 --- a/dlls/mshtml/nsembed.c +++ b/dlls/mshtml/nsembed.c @@ -41,6 +41,7 @@ WINE_DECLARE_DEBUG_CHANNEL(gecko); #define NS_APPSTARTUPNOTIFIER_CONTRACTID "@mozilla.org/embedcomp/appstartup-notifier;1" #define NS_WEBBROWSER_CONTRACTID "@mozilla.org/embedding/browser/nsWebBrowser;1" #define NS_COMMANDPARAMS_CONTRACTID "@mozilla.org/embedcomp/command-params;1" +#define NS_CYCLECOLLECTORLOGGER_CONTRACTID "@mozilla.org/cycle-collector-logger;1" #define NS_HTMLSERIALIZER_CONTRACTID "@mozilla.org/layout/contentserializer;1?mimetype=text/html" #define NS_EDITORCONTROLLER_CONTRACTID "@mozilla.org/editor/editorcontroller;1" #define NS_PREFERENCES_CONTRACTID "@mozilla.org/preferences;1" @@ -2201,6 +2202,119 @@ static const nsISupportsWeakReferenceVtbl nsSupportsWeakReferenceVtbl = { nsSupportsWeakReference_GetWeakReference };
+static nsICycleCollectorLogSink dummy_log_sink; + +static nsresult NSAPI dummy_log_sink_QueryInterface(nsICycleCollectorLogSink *iface, nsIIDRef riid, void **result) +{ + if(IsEqualGUID(&IID_nsISupports, riid) || IsEqualGUID(&IID_nsICycleCollectorLogSink, riid)) { + *result = &dummy_log_sink; + return NS_OK; + } + return NS_NOINTERFACE; +} + +static nsrefcnt NSAPI dummy_log_sink_AddRef(nsICycleCollectorLogSink *iface) +{ + return 2; +} + +static nsrefcnt NSAPI dummy_log_sink_Release(nsICycleCollectorLogSink *iface) +{ + return 1; +} + +static nsresult NSAPI dummy_log_sink_Open(nsICycleCollectorLogSink *iface, void **aGCLog, void **aCCLog) +{ + /* Always fail in Open, which disables the log entirely, while still forcing the CC */ + return NS_ERROR_NOT_IMPLEMENTED; +} + +static nsresult NSAPI dummy_log_sink_CloseGCLog(nsICycleCollectorLogSink *iface) +{ + return NS_OK; +} + +static nsresult NSAPI dummy_log_sink_CloseCCLog(nsICycleCollectorLogSink *iface) +{ + return NS_OK; +} + +static nsresult NSAPI dummy_log_sink_GetFilenameIdentifier(nsICycleCollectorLogSink *iface, nsAString *aIdentifier) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + +static nsresult NSAPI dummy_log_sink_SetFilenameIdentifier(nsICycleCollectorLogSink *iface, const nsAString *aIdentifier) +{ + return NS_OK; +} + +static nsresult NSAPI dummy_log_sink_GetProcessIdentifier(nsICycleCollectorLogSink *iface, LONG *aIdentifier) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + +static nsresult NSAPI dummy_log_sink_SetProcessIdentifier(nsICycleCollectorLogSink *iface, LONG aIdentifier) +{ + return NS_OK; +} + +static nsresult NSAPI dummy_log_sink_GetGcLog(nsICycleCollectorLogSink *iface, nsIFile **aPath) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + +static nsresult NSAPI dummy_log_sink_GetCcLog(nsICycleCollectorLogSink *iface, nsIFile **aPath) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + +static const nsICycleCollectorLogSinkVtbl dummy_log_sink_vtbl = { + dummy_log_sink_QueryInterface, + dummy_log_sink_AddRef, + dummy_log_sink_Release, + dummy_log_sink_Open, + dummy_log_sink_CloseGCLog, + dummy_log_sink_CloseCCLog, + dummy_log_sink_GetFilenameIdentifier, + dummy_log_sink_SetFilenameIdentifier, + dummy_log_sink_GetProcessIdentifier, + dummy_log_sink_SetProcessIdentifier, + dummy_log_sink_GetGcLog, + dummy_log_sink_GetCcLog +}; + +static nsICycleCollectorLogSink dummy_log_sink = { &dummy_log_sink_vtbl }; + +static void force_cycle_collection(nsIDOMWindowUtils *window_utils) +{ + nsICycleCollectorListener *logger, *all_logger; + nsresult nsres; + + /* Use a dummy logger to force the CC to clean up the entire graph, as in shutdown */ + nsres = nsIServiceManager_GetServiceByContractID(pServMgr, NS_CYCLECOLLECTORLOGGER_CONTRACTID, &IID_nsICycleCollectorListener, (void**)&logger); + if(NS_FAILED(nsres)) { + logger = NULL; + goto collect; + } + + nsres = nsICycleCollectorListener_AllTraces(logger, &all_logger); + nsICycleCollectorListener_Release(logger); + logger = NULL; + if(NS_FAILED(nsres)) + goto collect; + logger = all_logger; + + /* The dummy log sink will fail when opening the log, which will ignore it */ + nsres = nsICycleCollectorListener_SetLogSink(logger, &dummy_log_sink); + assert(nsres == NS_OK); + +collect: + nsIDOMWindowUtils_CycleCollect(window_utils, logger, 0); + if(logger) + nsICycleCollectorListener_Release(logger); +} + static HRESULT init_browser(GeckoBrowser *browser) { mozIDOMWindowProxy *mozwindow; @@ -2427,7 +2541,7 @@ void detach_gecko_browser(GeckoBrowser *This)
/* Force cycle collection */ if(window_utils) { - nsIDOMWindowUtils_CycleCollect(window_utils, NULL, 0); + force_cycle_collection(window_utils); nsIDOMWindowUtils_Release(window_utils); } } diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl index 2189e65a5d5..ccd1f5b82b0 100644 --- a/dlls/mshtml/nsiface.idl +++ b/dlls/mshtml/nsiface.idl @@ -181,7 +181,6 @@ typedef nsISupports nsILocalFile; typedef nsISupports nsIDOMHTMLMenuElement; typedef nsISupports nsIDOMCaretPosition; typedef nsISupports nsIFrameRequestCallback; -typedef nsISupports nsICycleCollectorListener; typedef nsISupports nsIDOMHTMLCanvasElement; typedef nsISupports nsIQueryContentEventResult; typedef nsISupports nsIDOMBlob; @@ -310,6 +309,43 @@ interface nsIThreadManager : nsISupports nsresult GetIsMainThread(bool *aResult); }
+[ + object, + uuid(3ad9875f-d0e4-4ac2-87e3-f127f6c02ce1), + local +] +interface nsICycleCollectorLogSink : nsISupports +{ + nsresult Open(void /*FILE*/ **aGCLog, void /*FILE*/ **aCCLog); + nsresult CloseGCLog(); + nsresult CloseCCLog(); + nsresult GetFilenameIdentifier(nsAString *aIdentifier); + nsresult SetFilenameIdentifier(const nsAString *aIdentifier); + nsresult GetProcessIdentifier(int32_t *aIdentifier); + nsresult SetProcessIdentifier(int32_t aIdentifier); + nsresult GetGcLog(nsIFile **aPath); + nsresult GetCcLog(nsIFile **aPath); +} + +[ + object, + uuid(703b53b6-24f6-40c6-9ea9-aeb2dc53d170), + local +] +interface nsICycleCollectorListener : nsISupports +{ + nsresult AllTraces(nsICycleCollectorListener **aListener); + nsresult GetWantAllTraces(bool *aAllTraces); + nsresult GetDisableLog(bool *aDisableLog); + nsresult SetDisableLog(bool aDisableLog); + nsresult GetLogSink(nsICycleCollectorLogSink **aLogSink); + nsresult SetLogSink(nsICycleCollectorLogSink *aLogSink); + nsresult GetWantAfterProcessing(bool *aWantAfterProcessing); + nsresult SetWantAfterProcessing(bool aWantAfterProcessing); + nsresult ProcessNext(void /*nsICycleCollectorHandler*/ *aHandler, bool *aCanContinue); + nsresult AsLogger(void /*nsCycleCollectorLogger*/ **aRetVal); +} + [ object, uuid(d1899240-f9d2-11d2-bdd6-000064657374),
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/nsembed.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c index 3ab256e0a0c..7f7a0a75131 100644 --- a/dlls/mshtml/nsembed.c +++ b/dlls/mshtml/nsembed.c @@ -2298,6 +2298,7 @@ static void force_cycle_collection(nsIDOMWindowUtils *window_utils) goto collect; }
+ /* Note that AllTraces logger merely sets a flag on the original, so it can't be re-used */ nsres = nsICycleCollectorListener_AllTraces(logger, &all_logger); nsICycleCollectorListener_Release(logger); logger = NULL; @@ -2313,6 +2314,19 @@ collect: nsIDOMWindowUtils_CycleCollect(window_utils, logger, 0); if(logger) nsICycleCollectorListener_Release(logger); + + /* Collection might have marked objects as SnowWhite, ensure they're cleaned up now, + but use a normal logger here that does *not* force the CC, to avoid wasting time. */ + nsres = nsIServiceManager_GetServiceByContractID(pServMgr, NS_CYCLECOLLECTORLOGGER_CONTRACTID, &IID_nsICycleCollectorListener, (void**)&logger); + if(NS_FAILED(nsres)) + logger = NULL; + else { + nsres = nsICycleCollectorListener_SetLogSink(logger, &dummy_log_sink); + assert(nsres == NS_OK); + } + nsIDOMWindowUtils_CycleCollect(window_utils, logger, 0); + if(logger) + nsICycleCollectorListener_Release(logger); }
static HRESULT init_browser(GeckoBrowser *browser)
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 | 118 +++++++++++++++++++++++++++++++++--------- 2 files changed, 94 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..2caf3e260c8 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,79 @@ 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); + + 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 +3578,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 +4052,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 +4076,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 2caf3e260c8..79e9be6d54d 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);
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132114
Your paranoid android.
=== debian11 (32 bit zh:CN report) ===
mshtml: events.c:2457: Test failed: complete = ffffffff
Ok, I found a problem. First, it appears that `unlink` isn't always called before `delete_cycle_collectable`. In case of no cycles, it may not be called at all, so this will leak the stuff that's unlinked.
This is easy enough solved by calling `unlink` from `delete_cycle_collectable`.
The next problem is that this causes a weird memory corruption, because now the `load_info` is actually released for channels in the non-cyclic cases, and this seems to cause the corruption. Presumably, this is why it was not released before. Any idea what's going on here?
I could drop the patch that releases the load_info, but I don't understand why it should be leaking? At least a comment would be warranted IMO.
Ok nevermind I found an already existing issue. It worked by pure luck now, because we were leaking the principals. `GetPrincipal` does **not** AddRef the returned principal, yet we released it, in `create_nsxhr`.
This is probably also the reason I had to add a hack in Proton for FFXIV Launcher since I used similar logic in `create_nsdomparser`, and explains the crash when we weren't leaking the document, since the principal was released.
I'll add the fix to this MR since it's related and trivial fix in hindsight…
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Marvin