[Bug 32951] New: Warlords III demo crashes on exit

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

Bug #: 32951
           Summary: Warlords III demo crashes on exit
           Product: Wine
           Version: 1.5.23
          Platform: x86
               URL: http://download.cnet.com/Warlords-III-demo/3000-2119_4
                    -10010589.html
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
    Classification: Unclassified

Created attachment 43520
  --> http://bugs.winehq.org/attachment.cgi?id=43520
Wine output, steps as described

To reproduce:
- install game
- start WAR3DEMO.EXE
- select Single player
- from the menu select Quit
Other than that the game works fine.
The crash backtrace is sometimes different, so there's probably a memory
corruption somewhere. The "expect problems" message is present each time.

[Bug 32951] Warlords III demo crashes on exit

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

Daniel Jelinski  changed:

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

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #1 from Henri Verbeet  2013-02-12 05:00:59 CST ---
(In reply to comment #0)
> The crash backtrace is sometimes different, so there's probably a memory
> corruption somewhere. The "expect problems" message is present each time.

Yeah, that message is harmless and mostly just wrong. Perhaps
WINEDEBUG=warn+heap will help if it's memory corruption. Also, is this a
regression?

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #2 from Daniel Jelinski  2013-02-12 13:31:53 CST ---
(In reply to comment #1)
> Yeah, that message is harmless and mostly just wrong. Perhaps
> WINEDEBUG=warn+heap will help if it's memory corruption. Also, is this a
> regression?

Indeed, doesn't crash (or at least doesn't run debugger) with warn+heap;
instead displays the following lines:
warn:heap:HEAP_ValidateInUseArena Heap 0x110000: invalid in-use arena magic
00555555 for 0x14c530
warn:heap:HEAP_ValidateInUseArena Heap 0x110000: invalid in-use arena magic
00555555 for 0x14e520
I also got segmentation fault once.

Not sure if that's a regression. There is little test data, but apparently the
crash was already present in 1.4-rc1
(http://appdb.winehq.org/objectManager.php?sClass=version&iId=3904&iT...).
I tested 1.5.4, and the crash is present there.

[Bug 32951] Warlords III demo crashes on exit

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

Bruno Jesus  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |[e-mail]
     Ever Confirmed|0                           |1

--- Comment #3 from Bruno Jesus  2013-02-13 19:57:36 CST ---
I can confirm this issue in wine 1.5.23. Most of the times I get 2 different
backtraces but more often I get this one:

=>0 0x00d4a843 (0x00f5e868)
  1 0x7e75682b ddraw_surface7_BltFast+0x3b(iface=<couldn't compute location>,
dstx=<couldn't compute location>, dsty=<couldn't compute location>,
Source=<couldn't compute location>, rsrc=<couldn't compute location>,
trans=<couldn't compute location>)
[/home/bruno/programs/wine32/dlls/ddraw/../../../wine/dlls/ddraw/surface.c:3922]
in ddraw (0x00f5e908)
  2 0x7e756f4d ddraw_surface1_BltFast+0xef(iface=<couldn't compute location>,
dst_x=<couldn't compute location>, dst_y=<couldn't compute location>,
src_surface=<couldn't compute location>, src_rect=<couldn't compute location>,
flags=<couldn't compute location>)
[/home/bruno/programs/wine32/dlls/ddraw/../../../wine/dlls/ddraw/surface.c:4038]
in ddraw (0x00f5e978)
  3 0x004abb14 in war3demo (+0xabb13) (0x006ba974)
0x00d4a843: addb        %al,0x0(%eax,%eax,1)

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #4 from Rico  2013-02-14 13:52:28 CST ---
Created attachment 43564
  --> http://bugs.winehq.org/attachment.cgi?id=43564
hack to prevent the crash

Somehow the game detaches the ddraw.dll and tries to use the interface.
Probably as long as some reference is left in the ddraw interface, we shouldn't
detach the dll? A test might bring in some light. Could you try the attached
patch?

/* DLL_PROCESS_DETACH */
0009:trace:ddraw:DllMain (0x7e700000,0,0x1)
0009:warn:ddraw:DllMain There are still existing DirectDraw interfaces. Wine
bug or buggy application?
0009:warn:ddraw:DllMain DDraw 0x12b6b0 has a refcount of 1
...
0009:trace:ddraw:ddraw7_SetCooperativeLevel iface 0x12b6b0, window (nil), flags
0x8.
0009:trace:ddraw:DDRAW_dump_cooperativelevel  - DDSCL_NORMAL 
0009:trace:ddraw:ddraw_attach_d3d_device ddraw 0x12b6b0.
0009:warn:ddraw:ddraw_attach_d3d_device No window for the Direct3DDevice,
created hidden window 0x30074.
0009:trace:ddraw:device_parent_create_swapchain device_parent 0x12b6d0, desc
0x32f884, swapchain 0x32f7b0.
0009:trace:ddraw:device_parent_create_swapchain_surface device_parent 0x12b6d0,
container_parent (nil), width 800, height 600, format_id 0xa, usage 0x1,
    multisample_type 0, multisample_quality 0, surface 0x13d410.
wine: Unhandled page fault on read access to 0x00000001 at address 0x7e73d199
(thread 0024), starting debugger...

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #5 from Henri Verbeet  2013-02-14 14:03:26 CST ---
(In reply to comment #4)
> Created attachment 43564 [details]
> hack to prevent the crash
>
> Somehow the game detaches the ddraw.dll and tries to use the interface.
> Probably as long as some reference is left in the ddraw interface, we shouldn't
> detach the dll? A test might bring in some light. Could you try the attached
> patch?

Well, in theory we prevent unloading the dll with the GetModuleHandleExW() call
a couple of lines above. Of course if the application just calls FreeLibrary()
in a loop, or something silly like that it's not going to help, and on process
exit everything is going to be unloaded anyway. Arguably we don't want to run
the cleanup code in either of those cases, so we can probably just remove it.
Still, it would probably be a good idea to verify that this isn't the result of
some refcounting bug somewhere.

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #6 from Rico  2013-02-14 14:16:36 CST ---
Created attachment 43565
  --> http://bugs.winehq.org/attachment.cgi?id=43565
log to show, that another thread still likes to draw ...

Attached is a log with backtrace. One thread frees the library and the other
draws ... that's the case why it is also kind of a random crash.

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #7 from Daniel Jelinski  2013-02-14 14:26:14 CST ---
Wine does not restore original screen resolution on exit when run with patch
from comment 4. Is that expected?

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #8 from Henri Verbeet  2013-02-14 14:34:28 CST ---
(In reply to comment #6)
> Created attachment 43565 [details]
> log to show, that another thread still likes to draw ...
>
> Attached is a log with backtrace. One thread frees the library and the other
> draws ... that's the case why it is also kind of a random crash.

How / why are we getting unloaded? The other thread being active seems to imply
it's not a normal unload due to process exit, and a single FreeLibrary()
shouldn't be enough to cause ddraw to get unloaded. Is there any chance the
other thread should have already been stopped instead?

(In reply to comment #7)
> Wine does not restore original screen resolution on exit when run with patch
> from comment 4. Is that expected?

Somewhat, the patch skips the cleanup code that takes care of that. It also
suggests the application doesn't cleanup itself either though.

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #9 from Rico  2013-02-14 14:59:11 CST ---
Created attachment 43566
  --> http://bugs.winehq.org/attachment.cgi?id=43566
backtrace on ddraw unload

I don't see a call to FreeLibrary() for the ddraw.dll at all. It happens on
shutdown. A bt while we get unloaded is attached.

To restore the screen just run:
xrandr -s 0

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #10 from Henri Verbeet  2013-02-14 15:20:02 CST ---
Right, ExitProcess() would probably do it. I think MSDN claims we're supposed
to kill other threads before unloading dlls, but I don't think we do. That's
not to say the result would necessarily be any better in general, but I suppose
in this case it would at least prevent the other thread from accessing surfaces
we just released.

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #11 from Rico  2013-02-14 15:45:55 CST ---
Yes, we don't terminate all threads before unloading the dlls. See
http://msdn.microsoft.com/en-us/library/windows/desktop/ms682658%28v=vs.85%2...
for reference.

Is a kernel/ntdll bug.

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #12 from Alexandre Julliard  2013-02-14 17:01:42 CST ---
Note that cleaning up refcounts or objects at process exit is a waste of time.
The only thing it really needs to do is restore screen resolution.

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #13 from Henri Verbeet  2013-02-14 17:25:13 CST ---
Yeah, though restoring the resolution is done by the swapchain on cleanup. We
can probably get away with just calling ddraw_destroy_swapchain(), but I'm not
entirely sure if that's going to be much better here, since at the very least
it's going to destroy the GL context as well. The original idea behind that
cleanup code in DllMain was probably more about detecting reference counting
problems inside ddraw itself, but the quality of the average ddraw application
makes it not really work out that way.

[Bug 32951] Warlords III demo crashes on exit

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

--- Comment #14 from Bruno Jesus  2013-04-21 11:37:11 CDT ---
Still in wine 1.5.28.

[Bug 32951] Warlords III demo crashes on exit

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

Daniel Jelinski  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |bfcdf6df8eea1230fb0e1835a80
                   |                            |9e49e5971e5d2
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #15 from Daniel Jelinski  2013-05-17 14:55:38 CDT ---
Fixed. Thanks Dmitry!

[Bug 32951] Warlords III demo crashes on exit

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

Alexandre Julliard  changed:

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

--- Comment #16 from Alexandre Julliard  2013-05-24 13:32:42 CDT ---
Closing bugs fixed in 1.5.31.

[Bug 32951] Warlords III demo crashes on exit

https://bugs.winehq.org/show_bug.cgi?id=32951

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
          Component|-unknown                    |kernel32