[2/7] DDraw: Protect IDirectDrawImpl against race conditions

Stefan Dösinger stefandoesinger at gmx.at
Fri Oct 13 04:01:31 CDT 2006


The same patch as yesterday, but an updated diff against the win64 fixes and 
the standard wine list fix.

Future patches protecting the surface, 3ddevice, ..., will lock the critical 
section of the ddraw object they belong to instead of having their own crit 
section to avoid locking inconsistencies.
-------------- next part --------------
From 5d13abdb0ecb82c7d984df19817db97e54b22ce6 Mon Sep 17 00:00:00 2001
From: Stefan Doesinger <stefan at codeweavers.com>
Date: Fri, 13 Oct 2006 10:50:26 +0200
Subject: [PATCH] DDraw: Protect IDirectDrawImpl against race conditions
---
 dlls/ddraw/ddraw.c         |  120 +++++++++++++++++++++++++++++++++++++++++---
 dlls/ddraw/ddraw_private.h |   12 ++++
 dlls/ddraw/direct3d.c      |   14 +++++
 dlls/ddraw/main.c          |    2 +
 dlls/ddraw/surface.c       |    2 +
 5 files changed, 139 insertions(+), 11 deletions(-)

diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index fb3559e..d5c7f87 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -140,6 +140,9 @@ IDirectDrawImpl_QueryInterface(IDirectDr
               IsEqualGUID( &IID_IDirect3D3 , refiid ) ||
               IsEqualGUID( &IID_IDirect3D7 , refiid ) )
     {
+        /* Lock because we might modify the ImplType and the d3dversion */
+        DDOBJ_LOCK(This);
+
         /* Check the surface implementation */
         if(This->ImplType == SURFACE_UNKNOWN)
         {
@@ -184,6 +187,7 @@ IDirectDrawImpl_QueryInterface(IDirectDr
             *obj = ICOM_INTERFACE(This, IDirect3D7);
             TRACE(" returning Direct3D7 interface at %p.\n", *obj);
         }
+        DDOBJ_UNLOCK(This);
     }
 
     /* Unknown interface */
@@ -241,10 +245,14 @@ IDirectDrawImpl_AddRef(IDirectDraw7 *ifa
 void
 IDirectDrawImpl_Destroy(IDirectDrawImpl *This)
 {
+    /* 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)
@@ -263,6 +271,13 @@ IDirectDrawImpl_Destroy(IDirectDrawImpl 
     IWineD3DDevice_Release(This->wineD3DDevice);
     IWineD3D_Release(This->wineD3D);
 
+    if(This->critName)
+    {
+        DeleteCriticalSection(&This->crit);
+        HeapFree(GetProcessHeap(), 0, This->critName);
+        This->critName = NULL;
+    }
+
     /* Now free the object */
     HeapFree(GetProcessHeap(), 0, This);
 }
@@ -301,6 +316,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
@@ -356,6 +373,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
@@ -461,6 +480,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
         return DDERR_INVALIDPARAMS;
     }
 
+    DDOBJ_LOCK(This);
     /* Handle those levels first which set various hwnds */
     if(cooplevel & DDSCL_SETFOCUSWINDOW)
     {
@@ -475,11 +495,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;
         }
@@ -504,6 +526,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;
         }
@@ -539,6 +562,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;
         }
@@ -576,6 +600,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
     }
     else if(cooplevel & DDSCL_EXCLUSIVE)
     {
+        DDOBJ_UNLOCK(This);
         TRACE("(%p) DDSCL_EXCLUSIVE needs DDSCL_FULLSCREEN\n", This);
         return DDERR_INVALIDPARAMS;
     }
@@ -599,13 +624,39 @@ 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);
+
+        /* DirectDraw critical section debug name:
+         * "ddraw_0xAAAAAAAAbbbbbbbb_crit"
+         * Size depens on the size of a pointer, 32 or 64 bit.
+         */
+        This->critName = HeapAlloc(GetProcessHeap(), 0, sizeof(This->critName[0]) * (16 + 2*sizeof(IDirectDrawImpl *)));;
+        if(!This->critName)
+        {
+            /* We did a nop-Lock before, do a nop unlock now to keep the code in a proper state */
+            DDOBJ_UNLOCK(This);
+            ERR("Out of memory when allocating a critical section name\n");
+            return DDERR_OUTOFMEMORY;
+        }
+        sprintf(This->critName, "ddraw_%p_crit", This);
+        InitializeCriticalSection(&This->crit);
+        This->crit.DebugInfo->Spare[0] = (DWORD_PTR) This->critName;
+
+        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)
@@ -613,6 +664,7 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
 
     /* Store the cooperative_level */
     This->cooperative_level |= cooplevel;
+    DDOBJ_UNLOCK(This);
     TRACE("SetCooperativeLevel retuning DD_OK\n");
     return DD_OK;
 }
@@ -629,6 +681,10 @@ IDirectDrawImpl_SetCooperativeLevel(IDir
  * It could mean that the current video mode should be left as-is. (But why
  * call it then?)
  *
+ * The msdn states that SetDisplayMode has to be called from the same thread
+ * that created the application window. Needs checking. What if there is no
+ * application window? (See above). That suggests that we need no locking.
+ *
  * Params:
  *  Height, Width: Screen dimension
  *  BPP: Color depth in Bits per pixel
@@ -718,6 +774,9 @@ IDirectDrawImpl_RestoreDisplayMode(IDire
     ICOM_THIS_FROM(IDirectDrawImpl, IDirectDraw7, iface);
     TRACE("(%p)\n", This);
 
+    /* No locking needed, the orig_* are initialized at creation time and
+     * never changed
+     */
     return IDirectDraw7_SetDisplayMode(ICOM_INTERFACE(This, IDirectDraw7),
                                        This->orig_width,
                                        This->orig_height,
@@ -748,6 +807,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) )
@@ -820,6 +880,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) 
@@ -942,8 +1003,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;
 }
 
@@ -967,6 +1030,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))
     {
@@ -1103,11 +1167,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;
 }
@@ -1130,6 +1196,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
@@ -1195,6 +1262,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
@@ -1390,6 +1458,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;
@@ -1407,7 +1477,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
@@ -1499,6 +1569,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,
@@ -1607,7 +1679,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
@@ -1642,6 +1716,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
@@ -1684,6 +1760,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
@@ -2147,6 +2227,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)
         {
@@ -2289,7 +2370,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 %08x\n", hr);
@@ -2330,10 +2413,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 */
@@ -2405,6 +2490,7 @@ IDirectDrawImpl_CreateSurface(IDirectDra
         WINED3DFORMAT Format;
         WINED3DPOOL Pool = WINED3DPOOL_DEFAULT;
 
+        DDOBJ_LOCK(This);
         This->tex_root = object;
 
         if(desc2.ddsCaps.dwCaps & DDSCAPS_MIPMAP)
@@ -2442,6 +2528,7 @@ IDirectDrawImpl_CreateSurface(IDirectDra
                                            (IUnknown *) ICOM_INTERFACE(object, IDirectDrawSurface7),
                                            D3D7CB_CreateSurface );
         This->tex_root = NULL;
+        DDOBJ_UNLOCK(This);
     }
 
     return hr;
@@ -2607,6 +2694,7 @@ IDirectDrawImpl_EnumSurfaces(IDirectDraw
     if(!Callback)
         return DDERR_INVALIDPARAMS;
 
+    DDOBJ_LOCK(This);
     /* Use the _SAFE enumeration, the app may destroy enumerated surfaces */
     LIST_FOR_EACH_SAFE(entry, entry2, &This->surface_list)
     {
@@ -2616,9 +2704,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;
 }
 
@@ -2628,7 +2720,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
@@ -2734,7 +2828,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,
@@ -2823,6 +2920,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),
@@ -2832,6 +2930,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
     {
@@ -2883,6 +2982,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,
@@ -2891,8 +2991,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 7181997..33d6b44 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -139,13 +139,19 @@ struct IDirectDrawImpl
     IDirectDrawSurfaceImpl *tex_root;
     BOOL                    depthstencil;
 
-    /* For the dll unload cleanup code */
+    /* For the dll unload cleanup code. Lock the global list critical section
+     * when using this field!
+     */
     struct list ddraw_list_entry;
     /* The surface list - can't relay this to WineD3D
      * because of IParent
      */
     struct list surface_list;
     LONG surfaces;
+
+    /* Multithreading synchronisation */
+    CRITICAL_SECTION        crit;
+    char                    *critName;
 };
 
 /* Declare the VTables. They can be found ddraw.c */
@@ -189,6 +195,10 @@ remove_ddraw_object(IDirectDrawImpl *ddr
 /* The default surface type */
 extern WINED3DSURFTYPE DefaultSurfaceType;
 
+/* Helper macro for multithreading, for all objects */
+#define DDOBJ_LOCK(obj) if(obj->critName) EnterCriticalSection(&obj->crit)
+#define DDOBJ_UNLOCK(obj) if(obj->critName) LeaveCriticalSection(&obj->crit)
+
 /*****************************************************************************
  * IDirectDrawSurface implementation structure
  *****************************************************************************/
diff --git a/dlls/ddraw/direct3d.c b/dlls/ddraw/direct3d.c
index 4a35938..2622784 100644
--- a/dlls/ddraw/direct3d.c
+++ b/dlls/ddraw/direct3d.c
@@ -741,9 +741,12 @@ 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
@@ -755,6 +758,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 +766,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 +793,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 +817,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 +858,11 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7 
                                                 This->d3d_target->WineD3DSurface,
                                                 target->WineD3DSurface);
         if(hr != D3D_OK)
-            ERR("(%p) Error %08x setting the front and back buffer\n", This, hr);
+        {
+            DDOBJ_UNLOCK(This);
+            ERR("(%p) Error %08lx setting the front and back buffer\n", This, hr);
+            return hr;
+        }
 
         object->OffScreenTarget = TRUE;
     }
@@ -872,6 +883,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 eec98f1..4a517c9 100644
--- a/dlls/ddraw/main.c
+++ b/dlls/ddraw/main.c
@@ -752,6 +752,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 d7940bd..58f32ff 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -238,7 +238,9 @@ static void IDirectDrawSurfaceImpl_Destr
 
     /* Reduce the ddraw surface count */
     InterlockedDecrement(&This->ddraw->surfaces);
+    DDOBJ_LOCK(This->ddraw);
     list_remove(&This->surface_list_entry);
+    DDOBJ_UNLOCK(This->ddraw);
 
     HeapFree(GetProcessHeap(), 0, This);
 }
-- 
1.4.1.1



More information about the wine-patches mailing list