[ 3/3] ddraw: Keep track of interface attached by AddAttachedSurface and detach correct interface when parent is released.

Octavian Voicu octavian.voicu at gmail.com
Mon Oct 10 01:07:41 CDT 2011


--
Fixes a bug that causes a stencil buffer leak in the game The Longest Journey.
Problem can be seen in the console output log attached to bug #11819.
This patch doesn't fix the original bug, but now game exits cleanly.

The root of the problem is that when releasing a surface, its attached
surfaces are detached using IDirectDraw7_DeleteAttachedSurface, regardless
of what interface was originally attached.

Here are the relevant lines from my own log, before the patch is applied:

trace:ddraw:ddraw_create_surface ddraw 0x1861e8, surface_desc 0x32ecf0, surface 0x32edc8, level 0.
trace:ddraw:ddraw_create_surface  (0x1861e8) Requesting surface desc :
trace:ddraw:DDRAW_dump_members  - DDSD_CAPS : DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_ZBUFFER
trace:ddraw:DDRAW_dump_members  - DDSD_HEIGHT : 480
trace:ddraw:DDRAW_dump_members  - DDSD_WIDTH : 640
trace:ddraw:DDRAW_dump_members  - DDSD_PIXELFORMAT : ( DDPF_ZBUFFER , Z bits : 16)
...
trace:d3d_surface:wined3d_surface_create Created surface 0x1bf6f0.
...
trace:ddraw:ddraw_create_surface Created surface 0x1bf5e0.
trace:ddraw:ddraw_surface1_AddAttachedSurface iface 0x1bc9f8, attachment 0x1bf5f0.
trace:ddraw:ddraw_surface3_AddAttachedSurface iface 0x1bc9f0, attachment 0x1bf5e8.
...
trace:ddraw:ddraw_surface_wined3d_object_destroyed surface 0x1bc9e8.
trace:ddraw:ddraw_surface7_DeleteAttachedSurface iface 0x1bc9e8, flags 0, attachment 0x1bf5e0.
trace:ddraw:ddraw_surface_delete_attached_surface surface 0x1bc9e8, attachment 0x1bf5e0.
trace:ddraw:ddraw_surface7_Release iface 0x1bf5e0 decreasing refcount to 4294967295.
...
trace:d3d:wined3d_device_uninit_3d Releasing depth/stencil buffer 0x1bf6f0.
trace:d3d_surface:wined3d_surface_decref Surface 0x1bf6f0, container (nil) of type 0.
trace:d3d_surface:wined3d_surface_decref 0x1bf6f0 decreasing refcount to 1.
err:d3d:wined3d_device_uninit_3d Something is still holding a reference to depth/stencil buffer 0x1bf6f0.
...
fixme:d3d:wined3d_device_decref Device released with resources still bound, acceptable but unexpected.
fixme:d3d:wined3d_device_decref Leftover resource 0x1bf6f0 with type WINED3DRTYPE_SURFACE (0x1).
---
 dlls/ddraw/ddraw_private.h  |    1 +
 dlls/ddraw/surface.c        |   58 +++++++++++++++++++++++++++++++++++-------
 dlls/ddraw/tests/refcount.c |    4 +-
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index e75acc0..da838e5 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -165,6 +165,7 @@ struct IDirectDrawSurfaceImpl
     /* This implementation handles attaching surfaces to other surfaces */
     IDirectDrawSurfaceImpl  *next_attached;
     IDirectDrawSurfaceImpl  *first_attached;
+    void                    *attached_iface;
 
     /* Complex surfaces are organized in a tree, although the tree is degenerated to a list in most cases.
      * In mipmap and primary surfaces each level has only one attachment, which is the next surface level.
diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 6140ee0..ef81ccd 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -1449,6 +1449,7 @@ static HRESULT WINAPI ddraw_surface7_AddAttachedSurface(IDirectDrawSurface7 *ifa
         return hr;
     }
     ddraw_surface7_AddRef(attachment);
+    attachment_impl->attached_iface = attachment;
     return hr;
 }
 
@@ -1468,6 +1469,7 @@ static HRESULT WINAPI ddraw_surface4_AddAttachedSurface(IDirectDrawSurface4 *ifa
     }
     ddraw_surface4_AddRef(attachment);
     ddraw_surface7_Release(&attachment_impl->IDirectDrawSurface7_iface);
+    attachment_impl->attached_iface = attachment;
     return hr;
 }
 static HRESULT WINAPI ddraw_surface3_AddAttachedSurface(IDirectDrawSurface3 *iface, IDirectDrawSurface3 *attachment)
@@ -1512,6 +1514,7 @@ static HRESULT WINAPI ddraw_surface3_AddAttachedSurface(IDirectDrawSurface3 *ifa
         return hr;
     }
     ddraw_surface3_AddRef(attachment);
+    attachment_impl->attached_iface = attachment;
     return hr;
 }
 
@@ -1531,6 +1534,7 @@ static HRESULT WINAPI ddraw_surface2_AddAttachedSurface(IDirectDrawSurface2 *ifa
     }
     ddraw_surface2_AddRef(attachment);
     ddraw_surface3_Release(&attachment_impl->IDirectDrawSurface3_iface);
+    attachment_impl->attached_iface = attachment;
     return hr;
 }
 
@@ -1550,6 +1554,7 @@ static HRESULT WINAPI ddraw_surface1_AddAttachedSurface(IDirectDrawSurface *ifac
     }
     ddraw_surface1_AddRef(attachment);
     ddraw_surface3_Release(&attachment_impl->IDirectDrawSurface3_iface);
+    attachment_impl->attached_iface = attachment;
     return hr;
 }
 
@@ -1624,6 +1629,10 @@ static HRESULT WINAPI ddraw_surface7_DeleteAttachedSurface(IDirectDrawSurface7 *
 
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
+    if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface7_iface)
+        WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+    attachment_impl->attached_iface = NULL;
+
     hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
     if (FAILED(hr))
     {
@@ -1642,6 +1651,10 @@ static HRESULT WINAPI ddraw_surface4_DeleteAttachedSurface(IDirectDrawSurface4 *
 
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
+    if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface4_iface)
+        WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+    attachment_impl->attached_iface = NULL;
+
     hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
     if (FAILED(hr))
     {
@@ -1659,6 +1672,10 @@ static HRESULT WINAPI ddraw_surface3_DeleteAttachedSurface(IDirectDrawSurface3 *
     HRESULT hr;
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
+    if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface3_iface)
+        WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+    attachment_impl->attached_iface = NULL;
+
     hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
     if (FAILED(hr))
     {
@@ -1676,6 +1693,10 @@ static HRESULT WINAPI ddraw_surface2_DeleteAttachedSurface(IDirectDrawSurface2 *
     HRESULT hr;
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
+    if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface2_iface)
+        WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+    attachment_impl->attached_iface = NULL;
+
     hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
     if (FAILED(hr))
     {
@@ -1693,6 +1714,10 @@ static HRESULT WINAPI ddraw_surface1_DeleteAttachedSurface(IDirectDrawSurface *i
     HRESULT hr;
     TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
 
+    if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface_iface)
+        WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+    attachment_impl->attached_iface = NULL;
+
     hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
     if (FAILED(hr))
     {
@@ -5091,6 +5116,27 @@ IDirectDrawSurfaceImpl *unsafe_impl_from_IDirect3DTexture(IDirect3DTexture *ifac
     return CONTAINING_RECORD(iface, IDirectDrawSurfaceImpl, IDirect3DTexture_iface);
 }
 
+static HRESULT delete_attached_surface_helper(IDirectDrawSurfaceImpl *root, IDirectDrawSurfaceImpl *detach)
+{
+    if (detach->attached_iface == &detach->IDirectDrawSurface4_iface)
+        return IDirectDrawSurface4_DeleteAttachedSurface(&root->IDirectDrawSurface4_iface, 0,
+                                                         &detach->IDirectDrawSurface4_iface);
+    else if (detach->attached_iface == &detach->IDirectDrawSurface3_iface)
+        return IDirectDrawSurface3_DeleteAttachedSurface(&root->IDirectDrawSurface3_iface, 0,
+                                                         &detach->IDirectDrawSurface3_iface);
+    else if (detach->attached_iface == &detach->IDirectDrawSurface2_iface)
+        return IDirectDrawSurface2_DeleteAttachedSurface(&root->IDirectDrawSurface2_iface, 0,
+                                                         &detach->IDirectDrawSurface2_iface);
+    else if (detach->attached_iface == &detach->IDirectDrawSurface_iface)
+        return IDirectDrawSurface_DeleteAttachedSurface(&root->IDirectDrawSurface_iface, 0,
+                                                        &detach->IDirectDrawSurface_iface);
+    else if (detach->attached_iface != &detach->IDirectDrawSurface7_iface)
+        FIXME("attached_iface %p is invalid for surface %p\n", detach->attached_iface, detach);
+
+    return IDirectDrawSurface7_DeleteAttachedSurface(&root->IDirectDrawSurface7_iface, 0,
+                                                     &detach->IDirectDrawSurface7_iface);
+}
+
 static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *parent)
 {
     IDirectDrawSurfaceImpl *surface = parent;
@@ -5100,9 +5146,6 @@ static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *paren
     /* Check for attached surfaces and detach them. */
     if (surface->first_attached != surface)
     {
-        IDirectDrawSurface7 *root = &surface->first_attached->IDirectDrawSurface7_iface;
-        IDirectDrawSurface7 *detach = &surface->IDirectDrawSurface7_iface;
-
         /* Well, this shouldn't happen: The surface being attached is
          * referenced in AddAttachedSurface(), so it shouldn't be released
          * until DeleteAttachedSurface() is called, because the refcount is
@@ -5111,18 +5154,13 @@ static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *paren
         WARN("Surface is still attached to surface %p.\n", surface->first_attached);
 
         /* The refcount will drop to -1 here */
-        if (FAILED(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach)))
+        if (FAILED(delete_attached_surface_helper(surface->first_attached, surface)))
             ERR("DeleteAttachedSurface failed.\n");
     }
 
     while (surface->next_attached)
-    {
-        IDirectDrawSurface7 *root = &surface->IDirectDrawSurface7_iface;
-        IDirectDrawSurface7 *detach = &surface->next_attached->IDirectDrawSurface7_iface;
-
-        if (FAILED(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach)))
+        if (FAILED(delete_attached_surface_helper(surface, surface->next_attached)))
             ERR("DeleteAttachedSurface failed.\n");
-    }
 
     /* Having a texture handle set implies that the device still exists. */
     if (surface->Handle)
diff --git a/dlls/ddraw/tests/refcount.c b/dlls/ddraw/tests/refcount.c
index 736ce8d..392054e 100644
--- a/dlls/ddraw/tests/refcount.c
+++ b/dlls/ddraw/tests/refcount.c
@@ -270,9 +270,9 @@ static void test_ddraw_objects(void)
             ref = IDirectDrawSurface_Release(surface1);
             ok(!ref, "Got refcount %d, expected 0\n", ref);
             ref = getRefcount( (IUnknown *) stencil1);
-            todo_wine ok(ref == 1, "Got refcount %d, expected 1\n", ref);
+            ok(ref == 1, "Got refcount %d, expected 1\n", ref);
             ref = IDirectDrawSurface_Release(stencil1);
-            todo_wine ok(!ref, "Got refcount %d, expected 0\n", ref);
+            ok(!ref, "Got refcount %d, expected 0\n", ref);
         }
         else
             IDirectDrawSurface_Release(surface1);
-- 
1.7.4.1




More information about the wine-patches mailing list