[PATCH v2 0/1] MR6864: mshtml: Don't use cycle collection for nsChannel.

[Gabriel Ivăncescu (＠insn)]

Tested and doesn't seem to be leaking anything extra.

--
  v2: mshtml: Don't use cycle collection for nsChannel.

https://gitlab.winehq.org/wine/wine/-/merge_requests/6864

[PATCH v2 1/1] mshtml: Don't use cycle collection for nsChannel.

[Gabriel Ivăncescu]

From: Gabriel Ivăncescu

This essentially reverts 668d8afd741b5827983569775abb56c0c914b5c8 (with
fixes since then), see MR !6859 for details.

Signed-off-by: Gabriel Ivăncescu 
---
 dlls/mshtml/binding.h |   2 +-
 dlls/mshtml/nsio.c    | 106 ++++++++++++------------------------------
 2 files changed, 30 insertions(+), 78 deletions(-)

diff --git a/dlls/mshtml/binding.h b/dlls/mshtml/binding.h
index 05db46db772..e9b7b218367 100644
--- a/dlls/mshtml/binding.h
+++ b/dlls/mshtml/binding.h
@@ -37,7 +37,7 @@ typedef struct {
     nsIHttpChannelInternal nsIHttpChannelInternal_iface;
     nsICacheInfoChannel    nsICacheInfoChannel_iface;

-    nsCycleCollectingAutoRefCnt ccref;
+    LONG ref;

nsWineURI *uri;
     nsIInputStream *post_data_stream;
diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c
index e42319dec13..a5564e1b6e4 100644
--- a/dlls/mshtml/nsio.c
+++ b/dlls/mshtml/nsio.c
@@ -45,7 +45,6 @@ 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"};
@@ -541,14 +540,6 @@ 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;
@@ -565,7 +556,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 = ccref_incr(&This->ccref, (nsISupports*)&This->nsIHttpChannel_iface);
+    nsrefcnt ref = InterlockedIncrement(&This->ref);

TRACE("(%p) ref=%ld\n", This, ref);

@@ -575,10 +566,36 @@ static nsrefcnt NSAPI nsChannel_AddRef(nsIHttpChannel *iface)
 static nsrefcnt NSAPI nsChannel_Release(nsIHttpChannel *iface)
 {
     nsChannel *This = impl_from_nsIHttpChannel(iface);
-    nsrefcnt ref = ccref_decr(&This->ccref, (nsISupports*)&This->nsIHttpChannel_iface, &nschannel_ccp);
+    nsrefcnt ref = InterlockedDecrement(&This->ref);

TRACE("(%p) ref=%ld\n", This, ref);

+    if(!ref) {
+        if(This->owner)
+            nsISupports_Release(This->owner);
+        if(This->post_data_stream)
+            nsIInputStream_Release(This->post_data_stream);
+        if(This->load_info)
+            nsISupports_Release(This->load_info);
+        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);
+
+        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);
+    }
+
     return ref;
 }

@@ -2248,64 +2265,6 @@ 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_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)
-        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);
-
-    unlink_ref(&This->owner);
-    unlink_ref(&This->post_data_stream);
-    unlink_ref(&This->load_info);
-    unlink_ref(&This->load_group);
-    unlink_ref(&This->notif_callback);
-    unlink_ref(&This->original_uri);
-    unlink_ref(&This->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 BOOL ensure_uri_builder(nsWineURI *This)
 {
     if(!This->is_mutable) {
@@ -3549,10 +3508,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;
@@ -4022,11 +3981,6 @@ 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;

@@ -4046,8 +4000,6 @@ 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)

[PATCH v2 0/1] MR6864: mshtml: Don't use cycle collection for nsChannel. - approved

[Jacek Caban (＠jacek)]

This merge request was approved by Jacek Caban.