[1/9] DDraw: Make IDirectDrawImpl thread safe

Stefan Dösinger stefandoesinger at gmx.at
Sat Aug 19 16:49:10 CDT 2006


This patch is the first patch of a series of patches that add critical 
sections to each ddraw implementation object to protect the members from race 
conditions. Basically the lock for an object is held if data is accessed that 
might change during the object lifetime. It is not locked when 'constant' 
data is used, like the wined3d interface, when Interlocked functions are 
used(AddRef, Release) or when only a single comparison is done(according to 
aj and rob this is not needed).

It does not protect wined3d data, this will be done in wined3d later, nor does 
it take care of the opengl context management, so applications still crash 
when they switch rendering threads.

If you find any problems with this patch please tell me, I'm fairly new to 
multithreading stuff.
-------------- next part --------------
From nobody Mon Sep 17 00:00:00 2001
From: Stefan Dösinger <stefan at codeweavers.com>
Date: Fri Aug 18 18:09:45 2006 +0200
Subject: [PATCH] DDraw: Make IDirectDrawImpl thread safe

---

 dlls/ddraw/ddraw.c         |  103 ++++++++++++++++++++++++++++++++++++++++----
 dlls/ddraw/ddraw_private.h |    8 +++
 dlls/ddraw/direct3d.c      |   15 ++++++
 dlls/ddraw/main.c          |    2 +
 dlls/ddraw/surface.c       |    2 +
 5 files changed, 120 insertions(+), 10 deletions(-)

a7fb979cf3ee3d6cbdb6be34748d324991a192a0
diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index 44fe5a0..a07d7dc 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -108,6 +108,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 ) )
@@ -193,10 +198,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;
 }
 
@@ -245,11 +252,12 @@ void
 IDirectDrawImpl_Destroy(IDirectDrawImpl *This)
 {
     IDirectDrawImpl *prev;
+    DDOBJ_LOCK(This);
 
     /* 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)
@@ -271,10 +279,16 @@ IDirectDrawImpl_Destroy(IDirectDrawImpl 
     else
     {
         for(prev = ddraw_list; prev; prev = prev->next)
+        {
             if(prev->next == This) break;
+        }
 
         if(prev)
+        {
+            DDOBJ_LOCK(prev);
             prev->next = This->next;
+            DDOBJ_UNLOCK(prev);
+        }
         else
             ERR("Didn't find the previous ddraw element in the list\n");
     }
@@ -283,6 +297,10 @@ IDirectDrawImpl_Destroy(IDirectDrawImpl 
     IWineD3DDevice_Release(This->wineD3DDevice);
     IWineD3D_Release(This->wineD3D);
 
+    DDOBJ_UNLOCK(This);
+    if(This->hasCrit) DeleteCriticalSection(&This->crit);
+    This->hasCrit = FALSE;
+
     /* Now free the object */
     HeapFree(GetProcessHeap(), 0, This);
 }
@@ -321,6 +339,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
@@ -376,6 +396,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
@@ -481,6 +503,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
         return DDERR_INVALIDPARAMS;
     }
 
+    DDOBJ_LOCK(This);
     /* Handle those levels first which set various hwnds */
     if(cooplevel & DDSCL_SETFOCUSWINDOW)
     {
@@ -495,11 +518,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;
         }
@@ -524,6 +549,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;
         }
@@ -556,6 +582,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;
         }
@@ -591,6 +618,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
     }
     else if(cooplevel & DDSCL_EXCLUSIVE)
     {
+        DDOBJ_UNLOCK(This);
         TRACE("(%p) DDSCL_EXCLUSIVE needs DDSCL_FULLSCREEN\n", This);
         return DDERR_INVALIDPARAMS;
     }
@@ -614,13 +642,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)
@@ -628,6 +667,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
 
     /* Store the cooperative_level */
     This->cooperative_level |= cooplevel;
+    DDOBJ_UNLOCK(This);
     TRACE("SetCooperativeLevel retuning DD_OK\n");
     return DD_OK;
 }
@@ -763,6 +803,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) )
@@ -835,6 +876,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) 
@@ -957,8 +999,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;
 }
 
@@ -982,6 +1026,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))
     {
@@ -1118,11 +1163,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;
 }
@@ -1145,6 +1192,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
@@ -1210,6 +1258,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
@@ -1405,6 +1454,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;
@@ -1422,7 +1473,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
@@ -1514,6 +1565,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,
@@ -1622,7 +1675,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
@@ -1657,6 +1712,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
@@ -1699,6 +1756,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
@@ -2151,6 +2212,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)
         {
@@ -2293,7 +2355,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);
@@ -2329,10 +2393,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 */
@@ -2341,11 +2407,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
@@ -2403,6 +2471,7 @@ IDirectDrawImpl_CreateSurface(IDirectDra
         WINED3DFORMAT Format;
         WINED3DPOOL Pool = WINED3DPOOL_DEFAULT;
 
+        DDOBJ_LOCK(This);
         This->tex_root = object;
 
         if(desc2.ddsCaps.dwCaps & DDSCAPS_MIPMAP)
@@ -2440,6 +2509,7 @@ IDirectDrawImpl_CreateSurface(IDirectDra
                                            (IUnknown *) ICOM_INTERFACE(object, IDirectDrawSurface7),
                                            D3D7CB_CreateSurface );
         This->tex_root = NULL;
+        DDOBJ_UNLOCK(This);
     }
 
     return hr;
@@ -2604,6 +2674,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)))
@@ -2611,9 +2682,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;
 }
 
@@ -2623,7 +2698,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
@@ -2729,7 +2806,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,
@@ -2818,6 +2898,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),
@@ -2827,6 +2908,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
     {
@@ -2878,6 +2960,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,
@@ -2886,8 +2969,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 5cf9312..e404e5e 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 */
@@ -188,6 +192,10 @@ extern IDirectDrawImpl *ddraw_list;
 /* 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 5c32ab6..287d4ad 100644
--- a/dlls/ddraw/main.c
+++ b/dlls/ddraw/main.c
@@ -729,6 +729,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 587a3e9..a55347f 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.2.4



More information about the wine-patches mailing list