[PATCH] ole32: Release marshal data before removing apartment from list.

[Sergio Gómez Del Real]

For local servers, we are first removing the apartment from the list and
then calling CoReleaseMarshalData(). This last function actually depends
on the apartment still being in the list (because of
apartment_findfromoxid()). The result is that CoReleaseMarshalData() is
failing, as can be seen in marshal.ok tests.
I only moved part of the code inside the critical section to keep it as
small as possible.

Signed-off-by: Sergio Gómez Del Real <sdelreal(a)codeweavers.com>
---
 dlls/ole32/compobj.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
index 66b1683d98..bae5941438 100644
--- a/dlls/ole32/compobj.c
+++ b/dlls/ole32/compobj.c
@@ -1169,6 +1169,7 @@ static void apartment_freeunusedlibraries(struct apartment *apt, DWORD delay)
 DWORD apartment_release(struct apartment *apt)
 {
     DWORD ret;
+    LocalServer *local_server = NULL;
 
     EnterCriticalSection(&csApartment);
 
@@ -1187,6 +1188,16 @@ DWORD apartment_release(struct apartment *apt)
         apt->being_destroyed = TRUE;
         if (apt == MTA) MTA = NULL;
         else if (apt == MainApartment) MainApartment = NULL;
+        /* we must release marshal data before removing apartment from its list */
+        if(apt->local_server) {
+            LARGE_INTEGER zero;
+            local_server = apt->local_server;
+
+            memset(&zero, 0, sizeof(zero));
+            IStream_Seek(local_server->marshal_stream, zero, STREAM_SEEK_SET, NULL);
+            CoReleaseMarshalData(local_server->marshal_stream);
+        }
+
         list_remove(&apt->entry);
     }
 
@@ -1198,13 +1209,8 @@ DWORD apartment_release(struct apartment *apt)
 
         TRACE("destroying apartment %p, oxid %s\n", apt, wine_dbgstr_longlong(apt->oxid));
 
-        if(apt->local_server) {
-            LocalServer *local_server = apt->local_server;
-            LARGE_INTEGER zero;
-
-            memset(&zero, 0, sizeof(zero));
-            IStream_Seek(local_server->marshal_stream, zero, STREAM_SEEK_SET, NULL);
-            CoReleaseMarshalData(local_server->marshal_stream);
+        if (local_server)
+        {
             IStream_Release(local_server->marshal_stream);
             local_server->marshal_stream = NULL;
 
-- 
2.17.1

[Huw Davies]

On Tue, Sep 11, 2018 at 06:04:09PM -0500, Sergio Gómez Del Real wrote:
> For local servers, we are first removing the apartment from the list and
> then calling CoReleaseMarshalData(). This last function actually depends
> on the apartment still being in the list (because of
> apartment_findfromoxid()). The result is that CoReleaseMarshalData() is
> failing, as can be seen in marshal.ok tests.
> I only moved part of the code inside the critical section to keep it as
> small as possible.

We're going to need a test for this.

Huw.
> Signed-off-by: Sergio Gómez Del Real <sdelreal(a)codeweavers.com>
> ---
>  dlls/ole32/compobj.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c
> index 66b1683d98..bae5941438 100644
> --- a/dlls/ole32/compobj.c
> +++ b/dlls/ole32/compobj.c
> @@ -1169,6 +1169,7 @@ static void apartment_freeunusedlibraries(struct apartment *apt, DWORD delay)
>  DWORD apartment_release(struct apartment *apt)
>  {
>      DWORD ret;
> +    LocalServer *local_server = NULL;
>
> EnterCriticalSection(&csApartment);
>
> @@ -1187,6 +1188,16 @@ DWORD apartment_release(struct apartment *apt)
>          apt->being_destroyed = TRUE;
>          if (apt == MTA) MTA = NULL;
>          else if (apt == MainApartment) MainApartment = NULL;
> +        /* we must release marshal data before removing apartment from its list */
> +        if(apt->local_server) {
> +            LARGE_INTEGER zero;
> +            local_server = apt->local_server;
> +
> +            memset(&zero, 0, sizeof(zero));
> +            IStream_Seek(local_server->marshal_stream, zero, STREAM_SEEK_SET, NULL);
> +            CoReleaseMarshalData(local_server->marshal_stream);
> +        }
> +
>          list_remove(&apt->entry);
>      }
>
> @@ -1198,13 +1209,8 @@ DWORD apartment_release(struct apartment *apt)
>
> TRACE("destroying apartment %p, oxid %s\n", apt, wine_dbgstr_longlong(apt->oxid));
>
> -        if(apt->local_server) {
> -            LocalServer *local_server = apt->local_server;
> -            LARGE_INTEGER zero;
> -
> -            memset(&zero, 0, sizeof(zero));
> -            IStream_Seek(local_server->marshal_stream, zero, STREAM_SEEK_SET, NULL);
> -            CoReleaseMarshalData(local_server->marshal_stream);
> +        if (local_server)
> +        {
>              IStream_Release(local_server->marshal_stream);
>              local_server->marshal_stream = NULL;
>
> -- 
> 2.17.1