d3d9: Don't release the parent device before destroying its children

Allan Tong actong88 at gmail.com
Mon Sep 28 09:02:41 CDT 2009


This fixes a regression in Last Remnant where it would crash on exit.
Releasing the last vertex declaration would free the D3D device before
the underlying wined3d vertex declaration had a chance to do its
cleanup.  Note that this partly reverts some recent commits that
changed the order of when the device is released:

789372afa82d0771196160dc538dd3d9791eb669
ca05ef5dd00385f46bb2fe94bce1f13d5860b7ae
66a723659060986ad3ce274e2555cc50958cd8d9
93b0600829e7f0906f4c5c73a13c409fccf1287b
a8e8f763bfa6674c803b8986722e3c40c962bd6b
de3bd86fb6eb9f6f6f15a6c8b2c338de6e442845
a286646f518847aa32e84cb04cd9fbb7a3378f46
a5214c306fd82982ed10fd8221ac00230186c9a6

I assume this was done to avoid referencing freed memory, but I'm not
sure if there was any other reason for this.

I see there's another patch on wine-patches that would also fix the
crash by having IWineD3DVertexDeclarationImpl_Release not access the
device anymore.  Generally, though, I still think it's wrong to
release the parent device before the child is done with its cleanup,
so I'm still sending this patch.

Note that this patch only changes d3d9.

 - Allan
-------------- next part --------------
From 446f2498a41d7fe6bc9c3950087e65c1dbbb2631 Mon Sep 17 00:00:00 2001
From: Allan Tong <actong88 at gmail.com>
Date: Sat, 26 Sep 2009 09:56:16 -0400
Subject: d3d9: Don't release the parent device before destroying its children.

Releasing the device earlier may cause the underlying wineD3D device
to be freed before the child object has had a chance to clean up.
---
 dlls/d3d9/cubetexture.c       |    6 +++++-
 dlls/d3d9/indexbuffer.c       |    6 +++++-
 dlls/d3d9/pixelshader.c       |    6 +++++-
 dlls/d3d9/surface.c           |    6 +++++-
 dlls/d3d9/swapchain.c         |    6 +++++-
 dlls/d3d9/texture.c           |    6 +++++-
 dlls/d3d9/vertexbuffer.c      |    6 +++++-
 dlls/d3d9/vertexdeclaration.c |    6 +++++-
 dlls/d3d9/vertexshader.c      |    6 +++++-
 dlls/d3d9/volumetexture.c     |    6 +++++-
 10 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/dlls/d3d9/cubetexture.c b/dlls/d3d9/cubetexture.c
index 4bab9df..10336b5 100644
--- a/dlls/d3d9/cubetexture.c
+++ b/dlls/d3d9/cubetexture.c
@@ -67,12 +67,16 @@ static ULONG WINAPI IDirect3DCubeTexture9Impl_Release(LPDIRECT3DCUBETEXTURE9 ifa
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         TRACE("Releasing child %p\n", This->wineD3DCubeTexture);
 
-        IDirect3DDevice9Ex_Release(This->parentDevice);
         wined3d_mutex_lock();
         IWineD3DCubeTexture_Release(This->wineD3DCubeTexture);
         wined3d_mutex_unlock();
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/indexbuffer.c b/dlls/d3d9/indexbuffer.c
index e089bea..2748abb 100644
--- a/dlls/d3d9/indexbuffer.c
+++ b/dlls/d3d9/indexbuffer.c
@@ -65,10 +65,14 @@ static ULONG WINAPI IDirect3DIndexBuffer9Impl_Release(LPDIRECT3DINDEXBUFFER9 ifa
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         wined3d_mutex_lock();
         IWineD3DBuffer_Release(This->wineD3DIndexBuffer);
         wined3d_mutex_unlock();
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/pixelshader.c b/dlls/d3d9/pixelshader.c
index 547dc74..1c42142 100644
--- a/dlls/d3d9/pixelshader.c
+++ b/dlls/d3d9/pixelshader.c
@@ -64,10 +64,14 @@ static ULONG WINAPI IDirect3DPixelShader9Impl_Release(LPDIRECT3DPIXELSHADER9 ifa
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         wined3d_mutex_lock();
         IWineD3DPixelShader_Release(This->wineD3DPixelShader);
         wined3d_mutex_unlock();
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/surface.c b/dlls/d3d9/surface.c
index f9b2794..0c277b9 100644
--- a/dlls/d3d9/surface.c
+++ b/dlls/d3d9/surface.c
@@ -82,10 +82,14 @@ static ULONG WINAPI IDirect3DSurface9Impl_Release(LPDIRECT3DSURFACE9 iface) {
         TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
         if (ref == 0) {
-            if (This->parentDevice) IDirect3DDevice9Ex_Release(This->parentDevice);
+            IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
             wined3d_mutex_lock();
             IWineD3DSurface_Release(This->wineD3DSurface);
             wined3d_mutex_unlock();
+
+            /* Release the device last, as it may cause the device to be destroyed. */
+            if (parentDevice) IDirect3DDevice9Ex_Release(parentDevice);
         }
 
         return ref;
diff --git a/dlls/d3d9/swapchain.c b/dlls/d3d9/swapchain.c
index e63e343..461db37 100644
--- a/dlls/d3d9/swapchain.c
+++ b/dlls/d3d9/swapchain.c
@@ -60,7 +60,8 @@ static ULONG WINAPI IDirect3DSwapChain9Impl_Release(LPDIRECT3DSWAPCHAIN9 iface)
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        if (This->parentDevice) IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         if (!This->isImplicit) {
             wined3d_mutex_lock();
             IWineD3DSwapChain_Destroy(This->wineD3DSwapChain);
@@ -68,6 +69,9 @@ static ULONG WINAPI IDirect3DSwapChain9Impl_Release(LPDIRECT3DSWAPCHAIN9 iface)
 
             HeapFree(GetProcessHeap(), 0, This);
         }
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        if (parentDevice) IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/texture.c b/dlls/d3d9/texture.c
index 9818c26..c63bb25 100644
--- a/dlls/d3d9/texture.c
+++ b/dlls/d3d9/texture.c
@@ -67,10 +67,14 @@ static ULONG WINAPI IDirect3DTexture9Impl_Release(LPDIRECT3DTEXTURE9 iface) {
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         wined3d_mutex_lock();
         IWineD3DTexture_Release(This->wineD3DTexture);
         wined3d_mutex_unlock();
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/vertexbuffer.c b/dlls/d3d9/vertexbuffer.c
index c46f8b3..d1d7b54 100644
--- a/dlls/d3d9/vertexbuffer.c
+++ b/dlls/d3d9/vertexbuffer.c
@@ -66,10 +66,14 @@ static ULONG WINAPI IDirect3DVertexBuffer9Impl_Release(LPDIRECT3DVERTEXBUFFER9 i
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         wined3d_mutex_lock();
         IWineD3DBuffer_Release(This->wineD3DVertexBuffer);
         wined3d_mutex_unlock();
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/vertexdeclaration.c b/dlls/d3d9/vertexdeclaration.c
index 1234bff..63b257c 100644
--- a/dlls/d3d9/vertexdeclaration.c
+++ b/dlls/d3d9/vertexdeclaration.c
@@ -248,10 +248,14 @@ static ULONG WINAPI IDirect3DVertexDeclaration9Impl_Release(LPDIRECT3DVERTEXDECL
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         if(!This->convFVF) {
             IDirect3DVertexDeclaration9Impl_Destroy(iface);
         }
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/vertexshader.c b/dlls/d3d9/vertexshader.c
index 8b775de..9293c20 100644
--- a/dlls/d3d9/vertexshader.c
+++ b/dlls/d3d9/vertexshader.c
@@ -64,10 +64,14 @@ static ULONG WINAPI IDirect3DVertexShader9Impl_Release(LPDIRECT3DVERTEXSHADER9 i
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         wined3d_mutex_lock();
         IWineD3DVertexShader_Release(This->wineD3DVertexShader);
         wined3d_mutex_unlock();
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
diff --git a/dlls/d3d9/volumetexture.c b/dlls/d3d9/volumetexture.c
index 3adab63..7a47e4a 100644
--- a/dlls/d3d9/volumetexture.c
+++ b/dlls/d3d9/volumetexture.c
@@ -66,10 +66,14 @@ static ULONG WINAPI IDirect3DVolumeTexture9Impl_Release(LPDIRECT3DVOLUMETEXTURE9
     TRACE("(%p) : ReleaseRef to %d\n", This, ref);
 
     if (ref == 0) {
-        IDirect3DDevice9Ex_Release(This->parentDevice);
+        IDirect3DDevice9Ex *parentDevice = This->parentDevice;
+
         wined3d_mutex_lock();
         IWineD3DVolumeTexture_Release(This->wineD3DVolumeTexture);
         wined3d_mutex_unlock();
+
+        /* Release the device last, as it may cause the device to be destroyed. */
+        IDirect3DDevice9Ex_Release(parentDevice);
     }
     return ref;
 }
-- 
1.6.0.4


More information about the wine-patches mailing list