[Bug 38241] New: Open General crashes when exiting.

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

Bug ID: 38241
           Summary: Open General crashes when exiting.
           Product: Wine
           Version: 1.7.38
          Hardware: x86
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: winmm&mci
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Created attachment 51048
  --> https://bugs.winehq.org/attachment.cgi?id=51048
WINEDEBUG=tid wine OpenGen.exe

Open General (strategy game) crashes when exiting after playing.
Might be the dupe of Bug 37427. However, I can't verify due to lack of
information. Terminal output is attached.

You can reproduce the issue with the following instructions.

Preparation:
1. Obtain Open General from http://sourceforge.net/projects/opengeneral/files/
. 
2. Extract and setup files, then run wine OpenGen.exe
3. Cancel the tutorial scenario by clicking [X] button.
4. Returning title screen, click the lower right [X] (Exit the game) button.

How to reproduce:
1. wine OpenGen.exe
2. Click [Play scenario] (the second button from the left)
3. In the popup dialog,
  i. Choose EFILE_OPENAK folder from [EFile folder] pulldown menu.
  ii. Select [Tutorial 1].
  iii. Click PC mark at P1: Faction B side.
  iv. Click the check-mark button at the lower right side.
4. Skip the equipments screen by clicking the check-mark button at the right
lower side.
5. Click the check-mark button to confirm.
6. Click the check-mark button and watching the battle.
7. Repeat 6 until finished.
8. Click the check-mark button.
9. Now, we back to title screen. Click [X] (Exit the game) button.
10. It crashes with an application error dialog.

[Bug 38241] Open General crashes when exiting.

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

Sagawa  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download
                URL|                            |sourceforge.net/projects/op
                   |                            |engeneral/files/
       Distribution|---                         |Ubuntu

[Bug 38241] Open General crashes when exiting.

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

--- Comment #1 from Sagawa  ---
Created attachment 51049
  --> https://bugs.winehq.org/attachment.cgi?id=51049
proposed patch
> From my inspection, this is MCI Mpegvideo driver, mciqtz32's issue.

Generally, mci "open" and "close" can call in different threads for the same
device.
Therefore, the following case triggers 'CoUninitialize Mismatch' in the current
implementation.
1. thread A, open foo.mp3 with MPEGVideo driver. i.e. 'open foo.mp3 alias foo
type MPEGVideo'. MCIQTZ_mciOpen() silently calls CoInitialize() for thread A.
2. thread A, play foo.
3. thread B, close foo. MCIQTZ_mciClose() implicitly calls CoUnintialize() for
thread B though the thread is not initialized.

Attached proposed patch introduces a task thread to avoid above situations, and
DriverProc passes the arguments to the thread.

[Bug 38241] Open General crashes when exiting.

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

--- Comment #2 from Bruno Jesus  ---
Unfortunately the patch no longer applies, if you rebase I can help testing.

[Bug 38241] Open General crashes when exiting.

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

Sagawa  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51049|0                           |1
        is obsolete|                            |

--- Comment #3 from Sagawa  ---
Created attachment 51322
  --> https://bugs.winehq.org/attachment.cgi?id=51322
proposed patch (rev.2)

I updated the patch. Could you try this patch?

[Bug 38241] Open General crashes when exiting.

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

Sagawa  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |46d5973961fe2266074ac285536
                   |                            |8c3fcf987c1b5
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Sagawa  ---
The patch was committed.
http://source.winehq.org/git/wine.git/commit/46d5973961fe2266074ac2855368c3f...

[Bug 38241] Open General crashes when exiting.

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

Alexandre Julliard  changed:

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

--- Comment #5 from Alexandre Julliard  ---
Closing bugs fixed in 1.7.43.

[Bug 38241] Open General crashes when exiting.

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

Andrew Eikum  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
                URL|sourceforge.net/projects/op |http://sourceforge.net/proj
                   |engeneral/files/            |ects/opengeneral/files/

[Bug 38241] Open General crashes when exiting.

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

--- Comment #6 from Andrew Eikum  ---
Created attachment 54385
  --> https://bugs.winehq.org/attachment.cgi?id=54385
winmm/tests: MCI rejects commands from threads that didn't open the device

(In reply to Sagawa from comment #1)
> Created attachment 51049 [details]
> proposed patch
>
> From my inspection, this is MCI Mpegvideo driver, mciqtz32's issue.
> Generally, mci "open" and "close" can call in different threads for the same
> device.
> Therefore, the following case triggers 'CoUninitialize Mismatch' in the
> current implementation.
> - thread A, open foo.mp3 with MPEGVideo driver. i.e. 'open foo.mp3 alias
>
> foo type MPEGVideo'. MCIQTZ_mciOpen() silently calls CoInitialize() for
> thread A.
> 2. thread A, play foo.
> 3. thread B, close foo. MCIQTZ_mciClose() implicitly calls CoUnintialize()
> for thread B though the thread is not initialized.
>
> Attached proposed patch introduces a task thread to avoid above situations,
> and DriverProc passes the arguments to the thread.

Some tests indicate this is wrong. MCI seems to reject commands from the thread
that didn't open them with MCIERR_INVALID_DEVICE_NAME. See attached tests, run
here:

https://testbot.winehq.org/JobDetails.pl?Key=22738

[Bug 38241] Open General crashes when exiting.

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

--- Comment #7 from Sagawa  ---
(In reply to Andrew Eikum from comment #6)
> Some tests indicate this is wrong. MCI seems to reject commands from the
> thread that didn't open them with MCIERR_INVALID_DEVICE_NAME. See attached
> tests, run here:
>
> https://testbot.winehq.org/JobDetails.pl?Key=22738

Yes, MCI seems to reject commands across threads on Windows NT, but not true on
Windows 9x. I'm afraid introduce some incompatibilities for them.
However, my approach introduces some issues as seen in Bug 40179. Now, I agree
with reverting 46d5973 and introduce thread checks. Thanks.

[Bug 38241] Open General crashes when exiting.

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

--- Comment #8 from Andrew Eikum  ---
(In reply to Sagawa from comment #7)
> (In reply to Andrew Eikum from comment #6)
> > Some tests indicate this is wrong. MCI seems to reject commands from the
> > thread that didn't open them with MCIERR_INVALID_DEVICE_NAME. See attached
> > tests, run here:
> >
> > https://testbot.winehq.org/JobDetails.pl?Key=22738
>
> Yes, MCI seems to reject commands across threads on Windows NT, but not true
> on Windows 9x. I'm afraid introduce some incompatibilities for them.
> However, my approach introduces some issues as seen in Bug 40179. Now, I
> agree with reverting 46d5973 and introduce thread checks. Thanks.

Thanks very much for the review and sign-off!