DDraw: Make IDirectDrawImpl thread safe

Stefan Dösinger stefandoesinger at gmx.at
Thu Oct 5 14:59:26 CDT 2006


This is a resend of the patch sent a few weeks ago. It protects the 
IDirectDrawImpl instances with a critical section if the application requests 
that with the DDSCL_MULTITHREADED cooperative level flag.

Patches for all other ddraw objects will follow once this patch is in to avoid 
flooding the list if this patch and likely all other patches need simmilar 
changes.
-------------- next part --------------
From 1692b4cd1a51dd09193bb09bbc0959aa15c1c598 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Stefan_D=F6singer?= <stefan at codeweavers.com>
Date: Sun, 27 Aug 2006 10:33:21 +0200
Subject: [PATCH] DDraw: Make IDirectDrawImpl thread safe
---
 dlls/ddraw/ddraw.c         |  105 ++++++++++++++++++++++++++++++++++++++++----
 dlls/ddraw/ddraw_private.h |    8 +++
 dlls/ddraw/direct3d.c      |   15 ++++++
 dlls/ddraw/main.c          |    2 +
 dlls/ddraw/surface.c       |    2 +
 5 files changed, 122 insertions(+), 10 deletions(-)

diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index 50b6738..75a6fe1 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -109,6 +109,11 @@ IDirectDrawImpl_QueryInterface(IDirectDr
     if(!refiid)
         return DDERR_INVALIDPARAMS;
 
+    /* Lock the object, we access and change private members when querying
+     * IDirect3D interfaces
+     */
+    DDOBJ_LOCK(This);
+
     /* Check DirectDraw Interfaces */
     if ( IsEqualGUID( &IID_IUnknown, refiid ) ||
          IsEqualGUID( &IID_IDirectDraw7, refiid ) )
@@ -194,10 +199,12 @@ IDirectDrawImpl_QueryInterface(IDirectDr
     else
     {
         ERR("(%p)->(%s, %p): No interface found\n", This, debugstr_guid(refiid), obj);
+        DDOBJ_UNLOCK(This);
         return E_NOINTERFACE;
     }
 
     IUnknown_AddRef( (IUnknown *) *obj );
+    DDOBJ_UNLOCK(This);
     return S_OK;
 }
 
@@ -246,11 +253,14 @@ void
 IDirectDrawImpl_Destroy(IDirectDrawImpl *This)
 {
     IDirectDrawImpl *prev;
+    /* No need to lock the object here, after the refcount falls to 0
+     * it shouldn't be in use anywere else
+     */
 
     /* Clear the cooplevel to restore window and display mode */
     IDirectDraw7_SetCooperativeLevel(ICOM_INTERFACE(This, IDirectDraw7),
-                                        NULL,
-                                        DDSCL_NORMAL);
+                                     NULL,
+                                     DDSCL_NORMAL);
 
     /* Destroy the device window if we created one */
     if(This->devicewindow != 0)
@@ -273,10 +283,17 @@ IDirectDrawImpl_Destroy(IDirectDrawImpl 
     else
     {
         for(prev = ddraw_list; prev; prev = prev->next)
+        {
             if(prev->next == This) break;
+        }
 
         if(prev)
+        {
+            /* No need to hold the lock on prev, the members related to the list
+             * are only modified / read when the whole list lock is held
+             */
             prev->next = This->next;
+        }
         else
             ERR("Didn't find the previous ddraw element in the list\n");
     }
@@ -286,6 +303,9 @@ IDirectDrawImpl_Destroy(IDirectDrawImpl 
     IWineD3DDevice_Release(This->wineD3DDevice);
     IWineD3D_Release(This->wineD3D);
 
+    if(This->hasCrit) DeleteCriticalSection(&This->crit);
+    This->hasCrit = FALSE;
+
     /* Now free the object */
     HeapFree(GetProcessHeap(), 0, This);
 }
@@ -324,6 +344,8 @@ IDirectDrawImpl_Release(IDirectDraw7 *if
  * Helper function that modifies a HWND's Style and ExStyle for proper
  * fullscreen use.
  *
+ * Assumes that the critical section is properly held
+ *
  * Params:
  *  This: Pointer to the DirectDraw implementation
  *  HWND: Window to setup
@@ -379,6 +401,8 @@ IDirectDrawImpl_SetupFullscreenWindow(ID
  * Helper function that restores a windows' properties when taking it out
  * of fullscreen mode
  *
+ * Assumes that the critical section is properly held
+ *
  * Params:
  *  This: Pointer to the DirectDraw implementation
  *  HWND: Window to setup
@@ -484,6 +508,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
         return DDERR_INVALIDPARAMS;
     }
 
+    DDOBJ_LOCK(This);
     /* Handle those levels first which set various hwnds */
     if(cooplevel & DDSCL_SETFOCUSWINDOW)
     {
@@ -498,11 +523,13 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
                          DDSCL_EXCLUSIVE       |
                          DDSCL_FULLSCREEN      ) )
         {
+            DDOBJ_UNLOCK(This);
             TRACE("Called with incompatible flags, returning DDERR_INVALIDPARAMS\n");
             return DDERR_INVALIDPARAMS;
         }
         else if( (This->cooperative_level & DDSCL_FULLSCREEN) && window)
         {
+            DDOBJ_UNLOCK(This);
             TRACE("Setting DDSCL_SETFOCUSWINDOW with an already set window, returning DDERR_HWNDALREADYSET\n");
             return DDERR_HWNDALREADYSET;
         }
@@ -527,6 +554,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
         /* Can't coexist with fullscreen or exclusive */
         if(cooplevel & (DDSCL_FULLSCREEN | DDSCL_EXCLUSIVE) )
         {
+            DDOBJ_UNLOCK(This);
             TRACE("(%p) DDSCL_NORMAL is not compative with DDSCL_FULLSCREEN or DDSCL_EXCLUSIVE\n", This);
             return DDERR_INVALIDPARAMS;
         }
@@ -562,6 +590,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
         /* Needs DDSCL_EXCLUSIVE */
         if(!(cooplevel & DDSCL_EXCLUSIVE) )
         {
+            DDOBJ_UNLOCK(This);
             TRACE("(%p) DDSCL_FULLSCREEN needs DDSCL_EXCLUSIVE\n", This);
             return DDERR_INVALIDPARAMS;
         }
@@ -599,6 +628,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
     }
     else if(cooplevel & DDSCL_EXCLUSIVE)
     {
+        DDOBJ_UNLOCK(This);
         TRACE("(%p) DDSCL_EXCLUSIVE needs DDSCL_FULLSCREEN\n", This);
         return DDERR_INVALIDPARAMS;
     }
@@ -622,13 +652,24 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
         }
     }
 
+    if(cooplevel & DDSCL_MULTITHREADED && !(This->cooperative_level & DDSCL_MULTITHREADED))
+    {
+        if(This->surfaces != 0)
+        {
+            ERR("Setting DDSCL_MULTITHREADED, but surfaces exist already\n");
+        }
+
+        TRACE("(%p): Creating a critical section for the IDirectDrawImpl object\n", This);
+        InitializeCriticalSection(&This->crit);
+        This->hasCrit = TRUE;
+        DDOBJ_LOCK(This); /* Lock to allow the unlock later to work */
+    }
+
     /* Unhandled flags */
     if(cooplevel & DDSCL_ALLOWREBOOT)
         WARN("(%p) Unhandled flag DDSCL_ALLOWREBOOT, harmless\n", This);
     if(cooplevel & DDSCL_ALLOWMODEX)
         WARN("(%p) Unhandled flag DDSCL_ALLOWMODEX, harmless\n", This);
-    if(cooplevel & DDSCL_MULTITHREADED)
-        FIXME("(%p) Unhandled flag DDSCL_MULTITHREADED, Uh Oh...\n", This);
     if(cooplevel & DDSCL_FPUSETUP)
         WARN("(%p) Unhandled flag DDSCL_FPUSETUP, harmless\n", This);
     if(cooplevel & DDSCL_FPUPRESERVE)
@@ -636,6 +677,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
 
     /* Store the cooperative_level */
     This->cooperative_level |= cooplevel;
+    DDOBJ_UNLOCK(This);
     TRACE("SetCooperativeLevel retuning DD_OK\n");
     return DD_OK;
 }
@@ -771,6 +813,7 @@ IDirectDrawImpl_GetCaps(IDirectDraw7 *if
 {
     ICOM_THIS_FROM(IDirectDrawImpl, IDirectDraw7, iface);
     TRACE("(%p)->(%p,%p)\n", This, DriverCaps, HELCaps);
+    /* The caps are only set at startup and not changed, no need to lock */
 
     /* One structure must be != NULL */
     if( (!DriverCaps) && (!HELCaps) )
@@ -843,6 +886,7 @@ IDirectDrawImpl_GetDisplayMode(IDirectDr
     WINED3DDISPLAYMODE Mode;
     DWORD Size;
     TRACE("(%p)->(%p): Relay\n", This, DDSD);
+    /* Does not use structure members, no need to lock */
 
     /* This seems sane */
     if(!DDSD) 
@@ -965,8 +1009,10 @@ IDirectDrawImpl_GetVerticalBlankStatus(I
     /* This looks sane, the MSDN suggests it too */
     if(!status) return DDERR_INVALIDPARAMS;
 
+    DDOBJ_LOCK(This);
     *status = This->fake_vblank;
     This->fake_vblank = !This->fake_vblank;
+    DDOBJ_UNLOCK(This);
     return DD_OK;
 }
 
@@ -990,6 +1036,7 @@ IDirectDrawImpl_GetAvailableVidMem(IDire
 {
     ICOM_THIS_FROM(IDirectDrawImpl, IDirectDraw7, iface);
     TRACE("(%p)->(%p, %p, %p)\n", This, Caps, total, free);
+    /* Total vidmem is const, free comes from wineD3D, no need to lock */
 
     if(TRACE_ON(ddraw))
     {
@@ -1126,11 +1173,13 @@ static HRESULT WINAPI IDirectDrawImpl_Ge
                                   &Mode);
 
     /* Fake the line sweeping of the monitor */
-    /* FIXME: We should synchronize with a source to keep the refresh rate */ 
+    /* FIXME: We should synchronize with a source to keep the refresh rate */
+    DDOBJ_LOCK(This);
     *Scanline = This->cur_scanline++;
     /* Assume 20 scan lines in the vertical blank */
     if (This->cur_scanline >= Mode.Height + 20)
         This->cur_scanline = 0;
+    DDOBJ_UNLOCK(This);
 
     return DD_OK;
 }
@@ -1153,6 +1202,7 @@ IDirectDrawImpl_TestCooperativeLevel(IDi
     ICOM_THIS_FROM(IDirectDrawImpl, IDirectDraw7, iface);
     HRESULT hr;
     TRACE("(%p)\n", This);
+    /* Only data from wineD3D is used, no need to lock */
 
     /* Description from MSDN:
      * For fullscreen apps return DDERR_NOEXCLUSIVEMODE if the user switched
@@ -1218,6 +1268,7 @@ IDirectDrawImpl_GetGDISurface(IDirectDra
     HRESULT hr;
     DDSCAPS2 ddsCaps;
     TRACE("(%p)->(%p)\n", This, GDISurface);
+    /* No need to lock, only data from wineD3D is used */
 
     /* Get the back buffer from the wineD3DDevice and search its
      * attached surfaces for the front buffer
@@ -1413,6 +1464,8 @@ IDirectDrawImpl_GetDeviceIdentifier(IDir
     /* The DDGDI_GETHOSTIDENTIFIER returns the information about the 2D
      * host adapter, if there's a secondary 3D adapter. This doesn't apply
      * to any modern hardware, nor is it interesting for Wine, so ignore it
+     *
+     * The device identifier is global and const, no need to lock anything
      */
 
     *DDDI = deviceidentifier;
@@ -1430,7 +1483,7 @@ IDirectDrawImpl_GetDeviceIdentifier(IDir
  *  Surface: Address to write the surface pointer to
  *
  * Returns:
- *  Always returns DD_OK because it's a stub
+ *  Always returns DDERR_NOTFOUND because it's a stub
  *
  *****************************************************************************/
 static HRESULT WINAPI
@@ -1522,6 +1575,8 @@ IDirectDrawImpl_StartModeTest(IDirectDra
  * Enumeration callback for IDirectDrawImpl_RecreateAllSurfaces.
  * It re-recreates the WineD3DSurface. It's pretty straightforward
  *
+ * Assumes that the lock is held properly by the caller
+ *
  *****************************************************************************/
 HRESULT WINAPI
 IDirectDrawImpl_RecreateSurfacesCallback(IDirectDrawSurface7 *surf,
@@ -1630,7 +1685,9 @@ IDirectDrawImpl_RecreateSurfacesCallback
  *
  * A function, that converts all wineD3DSurfaces to the new implementation type
  * It enumerates all surfaces with IWineD3DDevice::EnumSurfaces, creates a
- * new WineD3DSurface, copies the content and releases the old surface
+ * new WineD3DSurface, copies the content and releases the old surface.
+ *
+ * Assumes that the lock is held properly by the caller
  *
  *****************************************************************************/
 static HRESULT
@@ -1665,6 +1722,8 @@ IDirectDrawImpl_RecreateAllSurfaces(IDir
  * correct mipmap sublevel, and returns it to WineD3D.
  * The surfaces are created already by IDirectDraw7::CreateSurface
  *
+ * Assumes that the lock is held properly by IDirectDraw7::CreateSurface
+ *
  * Params:
  *  With, Height: With and height of the surface
  *  Format: The requested format
@@ -1707,6 +1766,10 @@ D3D7CB_CreateSurface(IUnknown *device,
  * A helper function for IDirectDraw7::CreateSurface. It creates a new surface
  * with the passed parameters.
  *
+ * Assumes that the lock on the IDirectDrawImpl object is held by
+ * IDirectDraw7::CreateSurface. No need to hold the lock on the surface,
+ * because it is beeing created
+ *
  * Params:
  *  DDSD: Description of the surface to create
  *  Surf: Address to store the interface pointer at
@@ -2167,6 +2230,7 @@ IDirectDrawImpl_CreateSurface(IDirectDra
                                        &Mode);
     if(FAILED(hr))
     {
+        /* This->orig_bpp is never changed, no need to lock */
         ERR("Failed to read display mode from wined3d\n");
         switch(This->orig_bpp)
         {
@@ -2309,7 +2373,9 @@ IDirectDrawImpl_CreateSurface(IDirectDra
     }
 
     /* Create the first surface */
+    DDOBJ_LOCK(This);
     hr = IDirectDrawImpl_CreateNewSurface(This, &desc2, &object, 0);
+    DDOBJ_UNLOCK(This);
     if( hr != DD_OK)
     {
         ERR("IDirectDrawImpl_CreateNewSurface failed with %08lx\n", hr);
@@ -2345,10 +2411,12 @@ IDirectDrawImpl_CreateSurface(IDirectDra
             desc2.dwHeight /= 2;
         }
 
+        DDOBJ_LOCK(This);
         hr = IDirectDrawImpl_CreateNewSurface(This,
                                               &desc2,
                                               &object2,
                                               level);
+        DDOBJ_UNLOCK(This);
         if(hr != DD_OK)
         {
             /* This destroys and possibly created surfaces too */
@@ -2357,11 +2425,13 @@ IDirectDrawImpl_CreateSurface(IDirectDra
         }
 
         /* Add the new surface to the complex attachment list */
+        DDOBJ_LOCK(This);
         object2->first_complex = object;
         object2->next_complex = NULL;
         iterator = object;
         while(iterator->next_complex) iterator = iterator->next_complex;
         iterator->next_complex = object2;
+        DDOBJ_UNLOCK(This);
 
         /* Remove the (possible) back buffer cap from the new surface description,
          * because only one surface in the flipping chain is a back buffer, one
@@ -2419,6 +2489,7 @@ IDirectDrawImpl_CreateSurface(IDirectDra
         WINED3DFORMAT Format;
         WINED3DPOOL Pool = WINED3DPOOL_DEFAULT;
 
+        DDOBJ_LOCK(This);
         This->tex_root = object;
 
         if(desc2.ddsCaps.dwCaps & DDSCAPS_MIPMAP)
@@ -2456,6 +2527,7 @@ IDirectDrawImpl_CreateSurface(IDirectDra
                                            (IUnknown *) ICOM_INTERFACE(object, IDirectDrawSurface7),
                                            D3D7CB_CreateSurface );
         This->tex_root = NULL;
+        DDOBJ_UNLOCK(This);
     }
 
     return hr;
@@ -2620,6 +2692,7 @@ IDirectDrawImpl_EnumSurfaces(IDirectDraw
     if(!Callback)
         return DDERR_INVALIDPARAMS;
 
+    DDOBJ_LOCK(This);
     for(surf = This->surface_list; surf; surf = surf->next)
     {
         if (all || (nomatch != IDirectDrawImpl_DDSD_Match(DDSD, &surf->surface_desc)))
@@ -2627,9 +2700,13 @@ IDirectDrawImpl_EnumSurfaces(IDirectDraw
             desc = surf->surface_desc;
             IDirectDrawSurface7_AddRef(ICOM_INTERFACE(surf, IDirectDrawSurface7));
             if(Callback( ICOM_INTERFACE(surf, IDirectDrawSurface7), &desc, Context) != DDENUMRET_OK)
+            {
+                DDOBJ_UNLOCK(This);
                 return DD_OK;
+            }
         }
     }
+    DDOBJ_UNLOCK(This);
     return DD_OK;
 }
 
@@ -2639,7 +2716,9 @@ IDirectDrawImpl_EnumSurfaces(IDirectDraw
  * Callback called by WineD3D to create Surfaces for render target usage
  * This function takes the D3D target from the IDirectDrawImpl structure,
  * and returns the WineD3DSurface. To avoid double usage, the surface
- * is marked as render target afterwards
+ * is marked as render target afterwards.
+ *
+ * Assumes that the lock is properly held by IDirectDraw7::CreateSurface
  *
  * Params
  *  device: The WineD3DDevice's parent
@@ -2745,7 +2824,10 @@ D3D7CB_CreateDepthStencilSurface(IUnknow
  * Callback function for WineD3D which creates a new WineD3DSwapchain
  * interface. It also creates an IParent interface to store that pointer,
  * so the WineD3DSwapchain has a parent and can be released when the D3D
- * device is destroyed
+ * device is destroyed.
+ *
+ * Assumes that the lock is properly held by IDirectDraw7::CreateSurface
+ *
  *****************************************************************************/
 static HRESULT WINAPI
 D3D7CB_CreateAdditionalSwapChain(IUnknown *device,
@@ -2834,6 +2916,7 @@ IDirectDrawImpl_AttachD3DDevice(IDirectD
     /* If there's no window, create a hidden window. WineD3D needs it */
     if(window == 0)
     {
+        DDOBJ_LOCK(This);
         window = CreateWindowExA(0, This->classname, "Hidden D3D Window",
                                  WS_DISABLED, 0, 0,
                                  GetSystemMetrics(SM_CXSCREEN),
@@ -2843,6 +2926,7 @@ IDirectDrawImpl_AttachD3DDevice(IDirectD
         ShowWindow(window, SW_HIDE);   /* Just to be sure */
         WARN("(%p) No window for the Direct3DDevice, created a hidden window. HWND=%p\n", This, window);
         This->d3d_window = window;
+        DDOBJ_UNLOCK(This);
     }
     else
     {
@@ -2894,6 +2978,7 @@ IDirectDrawImpl_AttachD3DDevice(IDirectD
     /* Set this NOW, otherwise creating the depth stencil surface will cause a
      * recursive loop until ram or emulated video memory is full
      */
+    DDOBJ_LOCK(This);
     This->d3d_initialized = TRUE;
 
     hr = IWineD3DDevice_Init3D(This->wineD3DDevice,
@@ -2902,8 +2987,10 @@ IDirectDrawImpl_AttachD3DDevice(IDirectD
     if(FAILED(hr))
     {
         This->wineD3DDevice = NULL;
+        DDOBJ_UNLOCK(This);
         return hr;
     }
+    DDOBJ_UNLOCK(This);
 
     /* Create an Index Buffer parent */
     TRACE("(%p) Successfully initialized 3D\n", This);
diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index abb6e36..ce269b3 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -145,6 +145,10 @@ struct IDirectDrawImpl
     /* For the dll unload cleanup code */
     IDirectDrawImpl *next;
     LONG surfaces;
+
+    /* Multithreading synchronisation */
+    CRITICAL_SECTION        crit;
+    BOOL                    hasCrit;
 };
 
 /* Declare the VTables. They can be found ddraw.c */
@@ -189,6 +193,10 @@ extern CRITICAL_SECTION listCrit;
 /* The default surface type */
 extern WINED3DSURFTYPE DefaultSurfaceType;
 
+/* Helper macro for multithreading, for all objects */
+#define DDOBJ_LOCK(obj) if(obj->hasCrit) EnterCriticalSection(&obj->crit)
+#define DDOBJ_UNLOCK(obj) if(obj->hasCrit) LeaveCriticalSection(&obj->crit)
+
 /*****************************************************************************
  * IDirectDrawSurface implementation structure
  *****************************************************************************/
diff --git a/dlls/ddraw/direct3d.c b/dlls/ddraw/direct3d.c
index ab59340..468f700 100644
--- a/dlls/ddraw/direct3d.c
+++ b/dlls/ddraw/direct3d.c
@@ -741,12 +741,16 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
 
     *Device = NULL;
 
+    /* Lock to be sure */
+    DDOBJ_LOCK(This);
     /* Fail device creation if non-opengl surfaces are used */
     if(This->ImplType != SURFACE_OPENGL)
     {
+        DDOBJ_UNLOCK(This);
         ERR("The application wants to create a Direct3D device, but non-opengl surfaces are set in the registry. Please set the surface implementation to opengl or autodetection to allow 3D rendering\n");
 
-        /* We only hit this path if a default surface is set in the registry. Incorrect autodetection
+        /* We only hit this path if a default s    DDOBJ_LOCK(This);
+        urface is set in the registry. Incorrect autodetection
          * is caught in CreateSurface or QueryInterface
          */
         return DDERR_NO3D;
@@ -755,6 +759,7 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
     /* So far we can only create one device per ddraw object */
     if(This->d3ddevice)
     {
+        DDOBJ_UNLOCK(This);
         FIXME("(%p): Only one Direct3D device per DirectDraw object supported\n", This);
         return DDERR_INVALIDPARAMS;
     }
@@ -762,6 +767,7 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
     object = HeapAlloc(GetProcessHeap(), 0, sizeof(IDirect3DDeviceImpl));
     if(!object)
     {
+        DDOBJ_UNLOCK(This);
         ERR("Out of memory when allocating a IDirect3DDevice implementation\n");
         return DDERR_OUTOFMEMORY;
     }
@@ -788,6 +794,7 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
     IndexBufferParent = HeapAlloc(GetProcessHeap(), 0, sizeof(IParentImpl *));
     if(!IndexBufferParent)
     {
+        DDOBJ_UNLOCK(This);
         ERR("Allocating memory for an index buffer parent failed\n");
         HeapFree(GetProcessHeap(), 0, object);
         return DDERR_OUTOFMEMORY;
@@ -811,6 +818,7 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
 
     if(FAILED(hr))
     {
+        DDOBJ_UNLOCK(This);
         ERR("Failed to create an index buffer\n");
         HeapFree(GetProcessHeap(), 0, object);
         return hr;
@@ -851,7 +859,11 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
                                                 This->d3d_target->WineD3DSurface,
                                                 target->WineD3DSurface);
         if(hr != D3D_OK)
+        {
+            DDOBJ_UNLOCK(This);
             ERR("(%p) Error %08lx setting the front and back buffer\n", This, hr);
+            return hr;
+        }
 
         object->OffScreenTarget = TRUE;
     }
@@ -872,6 +884,7 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
 
     This->d3ddevice = object;
 
+    DDOBJ_UNLOCK(This);
     return D3D_OK;
 }
 
diff --git a/dlls/ddraw/main.c b/dlls/ddraw/main.c
index 00ad213..a3a5f58 100644
--- a/dlls/ddraw/main.c
+++ b/dlls/ddraw/main.c
@@ -739,6 +739,8 @@ HRESULT WINAPI DllCanUnloadNow(void)
  * Callback function for the EnumSurfaces call in DllMain.
  * Dumps some surface info and releases the surface
  *
+ * No need to hold the lock because this is called when the last thread detaches
+ *
  * Params:
  *  surf: The enumerated surface
  *  desc: it's description
diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 111fa9e..4437faa 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -238,6 +238,7 @@ static void IDirectDrawSurfaceImpl_Destr
 
     /* Reduce the ddraw surface count */
     InterlockedDecrement(&This->ddraw->surfaces);
+    DDOBJ_LOCK(This->ddraw);
     if(This->prev)
         This->prev->next = This->next;
     else
@@ -247,6 +248,7 @@ static void IDirectDrawSurfaceImpl_Destr
     }
     if(This->next)
         This->next->prev = This->prev;
+    DDOBJ_UNLOCK(This->ddraw);
 
     HeapFree(GetProcessHeap(), 0, This);
 }
-- 
1.4.1.1



More information about the wine-patches mailing list