[Bug 10758] New: myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Summary: myst4 crashes during startup
           Product: Wine
           Version: 0.9.50.
          Platform: PC
               URL: http://mystworlds.ubi.com/us/mystiv_demo.php
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: wine-misc
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
                CC: [e-mail]

myst4 crashes during startup.  It started fine in 0.9.49.  Regression testing
suggests a problem with this patch:

e4f8a2da2b601964aa4261cb1617ba41f58b5a69 is first bad commit
commit e4f8a2da2b601964aa4261cb1617ba41f58b5a69
Author: Stefan Dösinger 
Date:   Sat Nov 10 00:19:19 2007 +0100

wined3d: Depth stencil fixes.

:040000 040000 87c42f53fd06ea5e0d79dccab11f605e6919d9e8
15eb241f3cda2c66c32b9c8100b405c43b74cce2 M      dlls

A demo is available from http://mystworlds.ubi.com/us/mystiv_demo.php.

I'm running Ubuntu 7.10 with nvidia drivers.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #1 from Stephen J. Smith   2007-12-12 18:35:58 ---
So here is a patch that makes Myst4 not crash for me:

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 7cd60c1..7e506b4 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -6045,7 +6045,8 @@ static HRESULT WINAPI
IWineD3DDeviceImpl_SetFrontBackBuffers(IWineD3DDevice *ifa

static HRESULT  WINAPI 
IWineD3DDeviceImpl_GetDepthStencilSurface(IWineD3DDevice* iface,
IWineD3DSurface **ppZStencilSurface) {
     IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
-    *ppZStencilSurface = This->stencilBufferTarget;
+    /* *ppZStencilSurface = This->stencilBufferTarget; */
+    *ppZStencilSurface = This->auto_depth_stencil_buffer;
     TRACE("(%p) : zStencilSurface  returning %p\n", This, 
*ppZStencilSurface);

if(*ppZStencilSurface != NULL) {

No clue if that is the right fix.  I thought of it because much of
e4f8a2da2b601964aa4261cb1617ba41f58b5a69 is about renaming depthStencilBuffer
to auto_depth_stencil_buffer, and because the last few things happening before
a crash are

0009:trace:d3d9:IDirect3DDevice9Impl_GetRenderTarget (0x1375b8) Relay
0009:trace:d3d:IWineD3DDeviceImpl_GetRenderTarget (0x157ad8) : RenderTarget 0
Index returning 0x1381e0
0009:trace:d3d_surface:IWineD3DBaseSurfaceImpl_AddRef (0x1381e0) : AddRef
increasing from 2
0009:trace:d3d_surface:IWineD3DBaseSurfaceImpl_GetParent (0x1381e0) : calling
resourceimpl_GetParent
0009:trace:d3d9:IDirect3DSurface9Impl_AddRef (0x1381b8)
0009:trace:d3d9:IDirect3DDevice9Impl_AddRef (0x1375b8) : AddRef from 1
0009:trace:d3d9:IDirect3DSurface9Impl_AddRef (0x1381b8) : AddRef from 0
0009:trace:d3d_surface:IWineD3DSurfaceImpl_Release (0x1381e0) : Releasing from
3
0009:trace:d3d9:IDirect3DDevice9Impl_GetDepthStencilSurface (0x1375b8) Relay
0009:trace:d3d:IWineD3DDeviceImpl_GetDepthStencilSurface (0x157ad8) :
zStencilSurface  returning 0x138360
0009:trace:d3d_surface:IWineD3DBaseSurfaceImpl_AddRef (0x138360) : AddRef
increasing from 1
0009:trace:d3d_surface:IWineD3DBaseSurfaceImpl_GetParent (0x138360) : calling
resourceimpl_GetParent
0009:trace:seh:raise_exception code=c0000005 flags=0 addr=0x138354

whereas the log from the version that doesn't crash looks like:

0009:trace:d3d9:IDirect3DDevice9Impl_GetRenderTarget (0x1375b8) Relay
0009:trace:d3d:IWineD3DDeviceImpl_GetRenderTarget (0x157ad8) : RenderTarget 0
Index returning 0x1381e0
0009:trace:d3d_surface:IWineD3DBaseSurfaceImpl_AddRef (0x1381e0) : AddRef
increasing from 2
0009:trace:d3d_surface:IWineD3DBaseSurfaceImpl_GetParent (0x1381e0) : calling
resourceimpl_GetParent
0009:trace:d3d9:IDirect3DSurface9Impl_AddRef (0x1381b8)
0009:trace:d3d9:IDirect3DDevice9Impl_AddRef (0x1375b8) : AddRef from 1
0009:trace:d3d9:IDirect3DSurface9Impl_AddRef (0x1381b8) : AddRef from 0
0009:trace:d3d_surface:IWineD3DSurfaceImpl_Release (0x1381e0) : Releasing from
3
0009:trace:d3d9:IDirect3DDevice9Impl_GetDepthStencilSurface (0x1375b8) Relay
0009:trace:d3d:IWineD3DDeviceImpl_GetDepthStencilSurface (0x157ad8) :
zStencilSurface  returning (nil)
0009:warn:d3d9:IDirect3DDevice9Impl_GetDepthStencilSurface Call to
IWineD3DDevice_GetDepthStencilSurface failed

Let me know what logs would be helpful.  The +all log from a crash is 53M.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #2 from Stefan Dösinger   2007-12-12 19:22:39 ---
No, the fix is not correct, you should not return the auto depth stencil buffer
when the function is supposed to return the currently set depth stencil buffer.

It would be interesting to see what happens with the depth stencil that is
returned(0x138360 in your crash log), and it's d3d9 parent. What might go on
here is that the current depth stencil buffer surface is destroyed(thus the
memory is freed), then the app calls GetDepthStencil and WineD3D attempts to
AddRef the destroyed surface and segfaults because the memory isn't valid any
longer.

If this is true, then it will need some test to see what is supposed to happen
and then fixed. I suspect that releasing the current depth stencil to ref 0
either sets the depth stencil to NULL, or delays destroying it until it is
unset.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #3 from Stephen J. Smith   2007-12-12 21:18:20 ---
Created an attachment (id=9613)
 --> (http://bugs.winehq.org/attachment.cgi?id=9613)
myst4 crash log

Attaching ~1000 lines (WINEDEBUG=+all) from around the crash.  Goes back well
before 0x138360 is first mentioned in the log.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Lei Zhang  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Lei Zhang  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #9613|text/x-log                  |text/plain
          mime type|                            |

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Frans Kool  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Cthulhu  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1

--- Comment #4 from Cthulhu   2008-09-03 01:54:34 ---
*** This bug has been confirmed by popular vote. ***

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Jim Cameron  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Jim Cameron   2008-10-01 17:27:44 ---
It is caused by a surface being freed where it shouldn't be ... I encountered
an interesting comment within IWineD3DDeviceImpl_SetDepthStencilSurface() (file
dlls/wined3d/device.c):

/* should we be calling the parent or the wined3d surface? */

followed by an AddRef of the wined3d surface. If I change the code to get the
parent and AddRef it instead, Revelations does not crash, and I could play the
game (although it crashed on exit). That would seem to answer the original
developer's question. Of course, we'll need a conformance test to confirm.
Patch follows.

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 409c7a1..0546f00 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -6648,7 +6648,12 @@ static HRESULT WINAPI
IWineD3DDeviceImpl_SetDepthStencilS
         tmp = This->stencilBufferTarget;
         This->stencilBufferTarget = pNewZStencil;
         /* should we be calling the parent or the wined3d surface? */
-        if (NULL != This->stencilBufferTarget)
IWineD3DSurface_AddRef(This->ste
+        if (NULL != This->stencilBufferTarget)
+        {
+            IUnknown* parent;
+            IWineD3DSurface_GetParent(This->stencilBufferTarget, &parent);
+            IUnknown_AddRef(parent);
+        }
         if (NULL != tmp) IWineD3DSurface_Release(tmp);
         hr = WINED3D_OK;

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #6 from Jim Cameron   2008-10-01 17:29:29 ---
Sorry, my terminal mangled that patch a bit. Here it is again.

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 409c7a1..0546f00 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -6648,7 +6648,12 @@ static HRESULT WINAPI
IWineD3DDeviceImpl_SetDepthStencilSurface(IWineD3DDevice *
         tmp = This->stencilBufferTarget;
         This->stencilBufferTarget = pNewZStencil;
         /* should we be calling the parent or the wined3d surface? */
-        if (NULL != This->stencilBufferTarget)
IWineD3DSurface_AddRef(This->stencilBufferTarget);
+        if (NULL != This->stencilBufferTarget)
+        {
+            IUnknown* parent;
+            IWineD3DSurface_GetParent(This->stencilBufferTarget, &parent);
+            IUnknown_AddRef(parent);
+        }
         if (NULL != tmp) IWineD3DSurface_Release(tmp);
         hr = WINED3D_OK;

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #7 from Jim Cameron   2008-10-03 06:06:23 ---
Leave aside for the moment the matter of the above patch haemorrhaging
surfaces...

The problem's a bit more complicated than this. The game is clearly expecting
SetDepthStencilSurface() to increase the reference count for the Direct3D 9
surface, which is an intuitively reasonable expectation. Since Wine's D3D9
implementation relies on the generic wined3d driver to do the heavy lifting,
only the backing WineD3D surface actually gets referenced.

The naive solution (of grabbing the parent and referencing it instead) doesn't
work with the auto depth stencil buffer, though. Wine D3D9 does not expect the
auto depth stencil buffer's D3D9 surface to be referenced merely by its being
used as a depth/stencil buffer, and if a D3D9 device with an auto depth stencil
buffer is reset, it will call SetDepthStencilSurface() on it, resulting in a
dangling reference that will block the /next/ attempt to reset the surface.
This causes Wine to fail several tests.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #8 from Jim Cameron   2008-10-05 11:00:02 ---
I've now been able to get onto a Windows machine with Direct3D 9 and done some
testing. My findings:

1. IDirect3DDevice9::SetDepthStencilSurface() does not increment the reference
count for the IDirect3DSurface9.
2. Releasing a IDirect3DSurface9 does not delete the surface if it is the
current depth-stencil surface, even if its reference count goes to zero.
3. IDirect3DDevice9::Reset() fails with a D3DERR_INVALIDCALL if an external
reference exists to the current depth-stencil surface.
4. IDirect3DDevice9::Reset() succeeds if no external references exist to the
current depth-stencil surface. In this case, the surface is deleted.
5. A user-supplied depth-stencil surface whose reference count is zero will be
deleted if a new surface, or NULL surface, is selected into the device using
IDirect3DDevice9::SetDepthStencilSurface(). The auto-depth-stencil surface will
not be deleted in the equivalent case.

The Wine bug is with number 2. Wine will not delete the auto-depth-stencil
surface if its refcount goes to zero because it is marked as implicit. But it
will happily delete a user-supplied depth-stencil surface when its refcount
goes to zero even if it is currently selected in a device.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #9 from Jim Cameron   2008-10-05 15:36:51 ---
Created an attachment (id=16488)
 --> (http://bugs.winehq.org/attachment.cgi?id=16488)
Keep D3D9 device's current depth-stencil surface from being deleted when its
refcount goes to 0

Attached is a proof-of-concept patch that fixes the problem with Myst IV. I've
added a private reference count to IDirect3DSurface9_Impl that is incremented
when a surface is passed to SetDepthStencilSurface() and decremented when it is
no longer required. Wine's Direct3D 9 tests pass with the patch.

I haven't included a test for this specific case, nor considered previous
Direct3D interface versions or other surface types. The private reference count
may also do strange things with implicit surfaces (the auto-depth-stencil
buffer). We should look into all these things if we do adopt this approach.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Xavier Vachon  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #10 from Xavier Vachon   2009-06-04 21:10:06 ---
Has there been any progress on this patch since? I have Myst 4 at home,
depending if there has been work done I could test it.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #11 from Ken Sharp   2009-06-05 17:00:58 ---
Test it and find out.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Lisa Denia  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from Lisa Denia   2009-07-04 07:53:05 ---
All solutions in this bugreport suffer from one big issue: The surfaces aren't
released correctly, making Revelation crash if you want to exit it. Thats not a
problem by itself(just not that beautiful...), but newer X.org server versions
crash nearly every time you exit Revelation. Yes, the entire X server.
This doesn't seem to be a regression in Wine. Could also be a bug in my NVidia
driver. But it proofs that all current solutions are unclean.
Also note the last few lines if you exit Revelation(which is possible without
crashing your X Server when running Revelation with virtual desktop enabled):

fixme:d3d:IWineD3DDeviceImpl_Uninit3D (0x14cb50) Something's still holding the
stencilBufferTarget
fixme:d3d:IWineD3DDeviceImpl_Release (0x14cb50) Device released with resources
still bound, acceptable but unexpected

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #13 from Henri Verbeet   2009-09-18 04:04:22 ---
How does this work with current git?

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #14 from Lisa Denia   2009-09-18 05:36:44 ---
Revelation starts up correctly using current git.

Maybe I should mention the following line in Wine's console output:
err:d3d:state_undefined Undefined state.
I don't think it has anything to do with your fix, but it wasn't there before.
Repeats quite often.
Haven't tested Revelation with the last few Wine releases though.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #15 from Henri Verbeet   2009-09-23 04:14:39 ---
Created an attachment (id=23716)
 --> (http://bugs.winehq.org/attachment.cgi?id=23716)
undefined state debug

Ok, so the bug itself is fixed then, right? You can use the attached patch to
figure out which state is causing the "Undefined state." FIXMEs. They're
usually harmless though.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

--- Comment #16 from Lisa Denia   2009-09-23 05:55:21 ---
Yup, the bug itself is fixed. It also seems everything gets released
correctly(at least the output mentioned in comment #12 isn't present anymore).

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #17 from Austin English   2009-09-23 10:36:52 ---
Fixed.

[Bug 10758] myst4 crashes during startup

http://bugs.winehq.org/show_bug.cgi?id=10758

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #18 from Alexandre Julliard   2009-09-25 12:24:06 ---
Closing bugs fixed in 1.1.30.