Re: [PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Andrew Eikum]

This seems like something you should be able to write a test for. I'm
also curious how this manages to work on Windows >= XP

You need to be more careful about thread shutdown. See
WINMM_StartDevicesThread and WINMM_DevicesThreadProc. I think you
could use the same logic, including WINMM_DevicesThreadDone, to
determine when to quit your new thread.

Andrew

On Thu, Dec 01, 2016 at 11:32:35AM -0200, Bruno Jesus wrote:
> try 2:
> Use a better way to detect Win9x (thanks Dmitry)
> Use a single point of config/teardown and a pointer to callback (thanks Nikolay)
>
> More detailed explanation in the patch comments. Basically the games use a DLL that calls SuspendThread from inside the WinMM callback, this makes the program hang as we use the "main thread" to dispatch callbacks.
>
> Games tested in Win95 config:
> Heroes of Might & Magic (well known affected)
> Deadlock (well known affected)
> Worms 2 (just to ensure things still work)
> Age of Empires (just to ensure things still work)
> Shivers (just to ensure things still work)
>
> Fixes bug https://bugs.winehq.org/show_bug.cgi?id=3930
> Signed-off-by: Bruno Jesus
>
> dlls/winmm/waveform.c | 117 ++++++++++++++++++++++++++++++++++++++++++--------
>  dlls/winmm/winemm.h   |   1 +
>  dlls/winmm/winmm.c    |   2 +-
>  3 files changed, 101 insertions(+), 19 deletions(-)
>
> diff --git a/dlls/winmm/waveform.c b/dlls/winmm/waveform.c
> index 7308519..4387c44 100644
> --- a/dlls/winmm/waveform.c
> +++ b/dlls/winmm/waveform.c
> @@ -134,6 +134,13 @@ struct _WINMM_MMDevice {
>      WINMM_Device *devices[MAX_DEVICES];
>  };
>
> +static struct {
> - HANDLE thread, event_main, event_thread;
> - WINMM_CBInfo *info;
> - WORD msg;
> - DWORD_PTR param1, param2;
>
> +} win9x;
> -
>
> static WINMM_MMDevice *g_out_mmdevices;
>  static WINMM_MMDevice **g_out_map;
>  static UINT g_outmmdevices_count;
> @@ -192,6 +199,77 @@ static LRESULT WID_Open(WINMM_OpenInfo *info);
>  static LRESULT WID_Close(HWAVEIN hwave);
>  static MMRESULT WINMM_BeginPlaying(WINMM_Device *device);
>
> +static void (*pWINMM_NotifyClient)(WINMM_CBInfo *info, WORD msg, DWORD_PTR param1,
> -
>    DWORD_PTR param2);
> -
> -
>
> +static DWORD WINAPI win9x_callback_thread(LPVOID param)
> +{
> - while(WaitForSingleObject(win9x.event_thread, INFINITE) == WAIT_OBJECT_0)
> - {
> -
>    if (!win9x.info->hwave) break;
> -
>    DriverCallback(win9x.info->callback, win9x.info->flags, (HDRVR)win9x.info->hwave,
> -
>                   win9x.msg, win9x.info->user, win9x.param1, win9x.param2);
> -
>    SetEvent(win9x.event_main);
> - }
> - return 0;
>
> +}
> -
>
> +/* Note: NotifyClient should never be called while holding the device lock
> -
> - since the client may call wave* functions from within the callback. */
>
> +static inline void WINMM_NotifyClient(WINMM_CBInfo *info, WORD msg, DWORD_PTR param1,
> -
>    DWORD_PTR param2)
>
> +{
> - DriverCallback(info->callback, info->flags, (HDRVR)info->hwave,
> -
>    msg, info->user, param1, param2);
>
> +}
> -
>
> +static inline void WINMM_NotifyClient9x(WINMM_CBInfo *info, WORD msg, DWORD_PTR param1,
> -
>    DWORD_PTR param2)
>
> +{
> - /* Prepare callback data for the thread, dispatch and wait response */
> - win9x.info = info;
> - win9x.msg = msg;
> - win9x.param1 = param1;
> - win9x.param2 = param2;
> - SetEvent(win9x.event_thread);
> - WaitForSingleObject(win9x.event_main, INFINITE);
>
> +}
> -
>
> +void WINMM_SetupCallback(void)
> +{
> - pWINMM_NotifyClient = WINMM_NotifyClient;
> - if (!(GetVersion() & 0x80000000))
> -
>    return;
> -
> - /* WAIL32.DLL from Rad Game Tools is an audio helper DLL that simplifies the
> -
> * use of sound effects in an application. It seems to be released in 1996 and
> -
> * is used at least by Heroes of Might and Magic and Deadlock. Unfortunately
> -
> * during a callback to the DLL it calls SuspendThread on the main thread and
> -
> * keeps doing some stuff from inside the callback until it does a ResumeThread
> -
> * and returns from the callback. This works in Windows 95/98 because they use
> -
> * a separate thread to send callback messages. XP changed this by going back
> -
> * to a single thread but the games still works due to shim.
> -
> */
> - TRACE("Using old behavior of WinMM callbacks\n");
> - win9x.event_thread = CreateEventA(NULL, FALSE, FALSE, NULL);
> - win9x.event_main = CreateEventA(NULL, FALSE, FALSE, NULL);
> - win9x.thread = CreateThread(NULL, 0, win9x_callback_thread, NULL, 0, NULL);
> - if (win9x.event_thread && win9x.event_main && win9x.thread)
> -
>    pWINMM_NotifyClient = WINMM_NotifyClient9x;
> - else
> - {
> -
>    ERR("Failed to create resources for callback, expect problems!\n");
> -
>    TerminateThread(win9x.thread, 0);
> -
>    CloseHandle(win9x.event_main);
> -
>    CloseHandle(win9x.event_thread);
> -
>    CloseHandle(win9x.thread);
> -
>    win9x.event_main = win9x.event_thread = win9x.thread = NULL;
> -
>    return;
> - }
>
> +}
> -
>
> +/* tear down function for DLL process detach */
>  void WINMM_DeleteWaveform(void)
>  {
>      UINT i, j;
> @@ -241,6 +319,18 @@ void WINMM_DeleteWaveform(void)
>      HeapFree(GetProcessHeap(), 0, g_device_handles);
>      HeapFree(GetProcessHeap(), 0, g_handle_devices);
> - if (win9x.thread)
> - {
> -
>    /* warn the thread so it can die gracefully */
> -
>    win9x.info->hwave = 0;
> -
>    SetEvent(win9x.event_thread);
> -
> -
>    CloseHandle(win9x.event_main);
> -
>    CloseHandle(win9x.event_thread);
> -
>    CloseHandle(win9x.thread);
> -
>    win9x.event_main = win9x.event_thread = win9x.thread = NULL;
> - }
> - DeleteCriticalSection(&g_devthread_lock);
>
> }
>
> @@ -370,15 +460,6 @@ static WINMM_Device *WINMM_GetDeviceFromHWAVE(HWAVE hwave)
>      return device;
>  }
>
> -/* Note: NotifyClient should never be called while holding the device lock
> -
> - since the client may call wave* functions from within the callback. */
>
> -static inline void WINMM_NotifyClient(WINMM_CBInfo *info, WORD msg, DWORD_PTR param1,
> -
>    DWORD_PTR param2)
>
> -{
> - DriverCallback(info->callback, info->flags, (HDRVR)info->hwave,
> -
>    msg, info->user, param1, param2);
>
> -}
> -
>
> static MMRESULT hr2mmr(HRESULT hr)
>  {
>      switch(hr){
> @@ -1753,7 +1834,7 @@ exit:
>          WAVEHDR *next = first->lpNext;
>          first->dwFlags &= ~WHDR_INQUEUE;
>          first->dwFlags |= WHDR_DONE;
> -
>    WINMM_NotifyClient(&cb_info, WOM_DONE, (DWORD_PTR)first, 0);
> -
>    pWINMM_NotifyClient(&cb_info, WOM_DONE, (DWORD_PTR)first, 0);
>    first = next;
> }
>
> }
> @@ -1929,7 +2010,7 @@ exit:
>              WAVEHDR *next = first->lpNext;
>              first->dwFlags &= ~WHDR_INQUEUE;
>              first->dwFlags |= WHDR_DONE;
> -
>        WINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)first, 0);
> -
>        pWINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)first, 0);
>        first = next;
>    }
> }
>
> @@ -2022,9 +2103,9 @@ static LRESULT WINMM_Reset(HWAVE hwave)
>          first->dwFlags &= ~WHDR_INQUEUE;
>          first->dwFlags |= WHDR_DONE;
>          if(is_out)
> -
>        WINMM_NotifyClient(&cb_info, WOM_DONE, (DWORD_PTR)first, 0);
> -
>        pWINMM_NotifyClient(&cb_info, WOM_DONE, (DWORD_PTR)first, 0);
>    else
> -
>        WINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)first, 0);
> -
>        pWINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)first, 0);
>    first = next;
> }
>
> @@ -2774,7 +2855,7 @@ MMRESULT WINAPI waveOutOpen(LPHWAVEOUT lphWaveOut, UINT uDeviceID,
>      cb_info.user = dwInstance;
>      cb_info.hwave = info.handle;
> - WINMM_NotifyClient(&cb_info, WOM_OPEN, 0, 0);
> -
>
> pWINMM_NotifyClient(&cb_info, WOM_OPEN, 0, 0);
>
> return res;
>
> }
> @@ -2802,7 +2883,7 @@ UINT WINAPI waveOutClose(HWAVEOUT hWaveOut)
>      res = SendMessageW(g_devices_hwnd, WODM_CLOSE, (WPARAM)hWaveOut, 0);
>  if(res == MMSYSERR_NOERROR)
> -
>    WINMM_NotifyClient(&cb_info, WOM_CLOSE, 0, 0);
> -
>    pWINMM_NotifyClient(&cb_info, WOM_CLOSE, 0, 0);
>
> return res;
>
> }
> @@ -3410,7 +3491,7 @@ MMRESULT WINAPI waveInOpen(HWAVEIN* lphWaveIn, UINT uDeviceID,
>      cb_info.user = dwInstance;
>      cb_info.hwave = info.handle;
> - WINMM_NotifyClient(&cb_info, WIM_OPEN, 0, 0);
> -
>
> pWINMM_NotifyClient(&cb_info, WIM_OPEN, 0, 0);
>
> return res;
>
> }
> @@ -3438,7 +3519,7 @@ UINT WINAPI waveInClose(HWAVEIN hWaveIn)
>      res = SendMessageW(g_devices_hwnd, WIDM_CLOSE, (WPARAM)hWaveIn, 0);
>  if(res == MMSYSERR_NOERROR)
> -
>    WINMM_NotifyClient(&cb_info, WIM_CLOSE, 0, 0);
> -
>    pWINMM_NotifyClient(&cb_info, WIM_CLOSE, 0, 0);
>
> return res;
>
> }
> @@ -3586,7 +3667,7 @@ UINT WINAPI waveInStop(HWAVEIN hWaveIn)
>      if(buf){
>          buf->dwFlags &= ~WHDR_INQUEUE;
>          buf->dwFlags |= WHDR_DONE;
> -
>    WINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)buf, 0);
> -
>    pWINMM_NotifyClient(&cb_info, WIM_DATA, (DWORD_PTR)buf, 0);
>
> }
>
> return MMSYSERR_NOERROR;
>
> diff --git a/dlls/winmm/winemm.h b/dlls/winmm/winemm.h
> index d7bab0f..10cd0f4 100644
> --- a/dlls/winmm/winemm.h
> +++ b/dlls/winmm/winemm.h
> @@ -151,6 +151,7 @@ void		TIME_MMTimeStop(void) DECLSPEC_HIDDEN;
>
> MMRESULT WINMM_CheckCallback(DWORD_PTR dwCallback, DWORD fdwOpen, BOOL mixer) DECLSPEC_HIDDEN;
>
> +void WINMM_SetupCallback(void) DECLSPEC_HIDDEN;
>  void WINMM_DeleteWaveform(void) DECLSPEC_HIDDEN;
>
> /* Global variables */
> diff --git a/dlls/winmm/winmm.c b/dlls/winmm/winmm.c
> index 0f3cd5d..358f139 100644
> --- a/dlls/winmm/winmm.c
> +++ b/dlls/winmm/winmm.c
> @@ -139,7 +139,7 @@ BOOL WINAPI DllMain(HINSTANCE hInstDLL, DWORD fdwReason, LPVOID fImpLoad)
>      if (!WINMM_CreateIData(hInstDLL))
>          return FALSE;
> -
> -
>    WINMM_SetupCallback();
>    break;
> case DLL_PROCESS_DETACH:
>        if(fImpLoad)
>
> -- 
> 2.9.3

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Bruno Jesus]

On Thu, Dec 1, 2016 at 12:11 PM, Andrew Eikum  wrote:
> This seems like something you should be able to write a test for. I'm
> also curious how this manages to work on Windows >= XP

The test is at [1], if you run in Win9x it will fail (Win9x uses
different thread), while running at >= XP it will succeed (>=XP uses
same thread). This works in XP due to a shim that blindly ignores the
SuspendThread/ResumeThread calls when inside a WinMM callback.

[1] http://source.winehq.org/source/dlls/winmm/tests/wave.c#0570
> You need to be more careful about thread shutdown. See
> WINMM_StartDevicesThread and WINMM_DevicesThreadProc. I think you
> could use the same logic, including WINMM_DevicesThreadDone, to
> determine when to quit your new thread.

I tried understand what you say but I'm not sure yet.
WINMM_DeleteWaveform is only called on process detach, can the DLL
live after that? What you mean is that I should call

GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
            (const WCHAR *)WINMM_StartDevicesThread, &g_9xthread_module);

inside WINMM_SetupCallback and then

FreeLibraryAndExitThread(g_9xthread_module, 1);

when returning from the thread?

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Andrew Eikum]

On Thu, Dec 01, 2016 at 12:27:43PM -0200, Bruno Jesus wrote:
> On Thu, Dec 1, 2016 at 12:11 PM, Andrew Eikum  wrote:
> > This seems like something you should be able to write a test for. I'm
> > also curious how this manages to work on Windows >= XP
>
> The test is at [1], if you run in Win9x it will fail (Win9x uses
> different thread), while running at >= XP it will succeed (>=XP uses
> same thread). This works in XP due to a shim that blindly ignores the
> SuspendThread/ResumeThread calls when inside a WinMM callback.
>
> [1] http://source.winehq.org/source/dlls/winmm/tests/wave.c#0570

See, I told you we could write a test for it. I already did ;)

I'm still not sure exactly how this shim works. Is that something we
can replicate in Wine instead of this threaded solution? Requiring
users to switch the Windows version isn't ideal. Does this game
require you to set 9x compatibility mode on Windows?
> > You need to be more careful about thread shutdown. See
> > WINMM_StartDevicesThread and WINMM_DevicesThreadProc. I think you
> > could use the same logic, including WINMM_DevicesThreadDone, to
> > determine when to quit your new thread.
>
> I tried understand what you say but I'm not sure yet.
> WINMM_DeleteWaveform is only called on process detach, can the DLL
> live after that?

The issue is that the callback thread may still be running after you
exit DllMain and the DLL is unloaded on a different thread, which will
cause a crash.

Because you can't use the Wait functions during DllMain, you need to
ensure no winmm code is being run on any thread *before*
PROCESS_DETACH is called. This means your thread needs to hold a
reference to the module to ensure it isn't unloaded. However, some
apps require winmm to be able to unload, so the thread can't just hold
the reference forever. Since your callback thread will only do useful
work if devices are opened (right?), you can use
WINMM_DevicesThreadDone to determine when to quit without depending on
Wait in DllMain. Finally, FreeLibraryAndExitThread allows you to exit
winmm code permanently in order to free the reference safely, in case
your thread is the last reference holder.

Note that this all means you'll need to start the callback thread
dynamically instead of just once. Since your thread and the existing
devices thread have the same lifespans (right?), I think you can just
piggyback on WINMM_StartDevicesThread and start both threads in there.

Also, double-check that g_devthread_token is incremented in all
instances where your callback thread may be used while no devices
exist. This situation might never occur, but double-check to make
sure.

I thought we had a bug about this, but I can't find it. See commit
2d76befbddc798f02812fe48a12ab2b80d25181b.

Hope that makes sense, it took a while to wrap my head around it when
I wrote that commit...

Andrew

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Bruno Jesus]

On Thu, Dec 1, 2016 at 1:14 PM, Andrew Eikum  wrote:
> On Thu, Dec 01, 2016 at 12:27:43PM -0200, Bruno Jesus wrote:
> > On Thu, Dec 1, 2016 at 12:11 PM, Andrew Eikum  wrote:
> > > This seems like something you should be able to write a test for. I'm
> > > also curious how this manages to work on Windows >= XP
> >
> > The test is at [1], if you run in Win9x it will fail (Win9x uses
> > different thread), while running at >= XP it will succeed (>=XP uses
> > same thread). This works in XP due to a shim that blindly ignores the
> > SuspendThread/ResumeThread calls when inside a WinMM callback.
> >
> > [1] http://source.winehq.org/source/dlls/winmm/tests/wave.c#0570
>
> See, I told you we could write a test for it. I already did ;)

Sorry for not saying before that the test previously existed.
> I'm still not sure exactly how this shim works. Is that something we
> can replicate in Wine instead of this threaded solution? Requiring
> users to switch the Windows version isn't ideal. Does this game
> require you to set 9x compatibility mode on Windows?

No, manually setting compatibility is not required in XP (that is why
IMO its is a shim, more below). Just the game installer needs
compatibility or it complains about unsupported OS.

I could not find any way to disable SuspendThread, I tried it back in
[1] but got no replies, internet also did not help me at this subject.

[1] https://www.winehq.org/pipermail/wine-devel/2015-November/110308.html

To prove my point at [2] (source at [3]) there is a winmm_crosstest
that calls SuspendThread inside the callback. If you run this at
Windows >= XP it will get stuck. If you turn compatibility mode on it
will pass as if nothing was called (tested on XP and 8).

[2] http://alexa.pro.br/~bruno/wine/winmm_crosstest.exe
[3] http://alexa.pro.br/~bruno/wine/winmm.txt
> > > You need to be more careful about thread shutdown. See
> > > WINMM_StartDevicesThread and WINMM_DevicesThreadProc. I think you
> > > could use the same logic, including WINMM_DevicesThreadDone, to
> > > determine when to quit your new thread.
> >
> > I tried understand what you say but I'm not sure yet.
> > WINMM_DeleteWaveform is only called on process detach, can the DLL
> > live after that?
>
> The issue is that the callback thread may still be running after you
> exit DllMain and the DLL is unloaded on a different thread, which will
> cause a crash.

Makes sense to me.
> Because you can't use the Wait functions during DllMain, you need to
> ensure no winmm code is being run on any thread *before*
> PROCESS_DETACH is called. This means your thread needs to hold a
> reference to the module to ensure it isn't unloaded. However, some
> apps require winmm to be able to unload, so the thread can't just hold
> the reference forever. Since your callback thread will only do useful
> work if devices are opened (right?), you can use
> WINMM_DevicesThreadDone to determine when to quit without depending on
> Wait in DllMain. Finally, FreeLibraryAndExitThread allows you to exit
> winmm code permanently in order to free the reference safely, in case
> your thread is the last reference holder.

Yes, the thread is only useful when devices are opened.
> Note that this all means you'll need to start the callback thread
> dynamically instead of just once. Since your thread and the existing
> devices thread have the same lifespans (right?), I think you can just
> piggyback on WINMM_StartDevicesThread and start both threads in there.

You are right, the lifespans are the same.
> Also, double-check that g_devthread_token is incremented in all
> instances where your callback thread may be used while no devices
> exist. This situation might never occur, but double-check to make
> sure.
>
> I thought we had a bug about this, but I can't find it. See commit
> 2d76befbddc798f02812fe48a12ab2b80d25181b.
>
> Hope that makes sense, it took a while to wrap my head around it when
> I wrote that commit...

Thanks, it all makes sense. I have to study further now.
> Andrew

Thank you very much again.

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Andrew Eikum]

On Thu, Dec 01, 2016 at 02:24:38PM -0200, Bruno Jesus wrote:
> No, manually setting compatibility is not required in XP (that is why
> IMO its is a shim, more below). Just the game installer needs
> compatibility or it complains about unsupported OS.
>
> I could not find any way to disable SuspendThread, I tried it back in
> [1] but got no replies, internet also did not help me at this subject.
>
> [1] https://www.winehq.org/pipermail/wine-devel/2015-November/110308.html
>
> To prove my point at [2] (source at [3]) there is a winmm_crosstest
> that calls SuspendThread inside the callback. If you run this at
> Windows >= XP it will get stuck. If you turn compatibility mode on it
> will pass as if nothing was called (tested on XP and 8).
>
> [2] http://alexa.pro.br/~bruno/wine/winmm_crosstest.exe
> [3] http://alexa.pro.br/~bruno/wine/winmm.txt

If the game works without compat mode, but your test doesn't, then
there must be something else going on here. I wish we understood
better why it works.

Andrew

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Bruno Jesus]

On Thu, Dec 1, 2016 at 3:29 PM, Andrew Eikum  wrote:
> On Thu, Dec 01, 2016 at 02:24:38PM -0200, Bruno Jesus wrote:
> > No, manually setting compatibility is not required in XP (that is why
> > IMO its is a shim, more below). Just the game installer needs
> > compatibility or it complains about unsupported OS.
> >
> > I could not find any way to disable SuspendThread, I tried it back in
> > [1] but got no replies, internet also did not help me at this subject.
> >
> > [1] https://www.winehq.org/pipermail/wine-devel/2015-November/110308.html
> >
> > To prove my point at [2] (source at [3]) there is a winmm_crosstest
> > that calls SuspendThread inside the callback. If you run this at
> > Windows >= XP it will get stuck. If you turn compatibility mode on it
> > will pass as if nothing was called (tested on XP and 8).
> >
> > [2] http://alexa.pro.br/~bruno/wine/winmm_crosstest.exe
> > [3] http://alexa.pro.br/~bruno/wine/winmm.txt
>
> If the game works without compat mode, but your test doesn't, then
> there must be something else going on here. I wish we understood
> better why it works.

I don't know what else to do, you can read some more at
https://bugs.winehq.org/show_bug.cgi?id=3930#c18

You can also read the changelog of the DLL file
http://www.radgametools.com/msshist.htm, especifically this (which
could or not be related to the problem, and the games were shipped
with old versions.):
---------
Changes from 3.03c to 3.03d (11-22-1995)

Switched to a new thread synchronization technique which should fix
deadlocking problems previously encountered by some 32-bit MSS
applications.
---------

It would not be the first time Windows circumvents games problems. My
patch is only touching Wine as configured for 9x, it brings no issues
to the general user and makes Wine behave as 9x did. I cannot solve
the mistery of how XP detects the game and avoids SuspendThread as my
knowledge is limited and I believe it would require reverse
engineering.

Best wishes,
Bruno

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Andrew Eikum]

On Thu, Dec 01, 2016 at 04:04:42PM -0200, Bruno Jesus wrote:
> On Thu, Dec 1, 2016 at 3:29 PM, Andrew Eikum  wrote:
> > If the game works without compat mode, but your test doesn't, then
> > there must be something else going on here. I wish we understood
> > better why it works.
>
> I don't know what else to do, you can read some more at
> https://bugs.winehq.org/show_bug.cgi?id=3930#c18

Huh, interesting. Either Windows has magic for this game, or it uses
some other heuristic to determine whether to use the old behavior.
> It would not be the first time Windows circumvents games problems. My
> patch is only touching Wine as configured for 9x, it brings no issues
> to the general user and makes Wine behave as 9x did. I cannot solve
> the mistery of how XP detects the game and avoids SuspendThread as my
> knowledge is limited and I believe it would require reverse
> engineering.

To be clear, I'm not rejecting your idea. Whatever this heuristic is,
it would just replace the Windows version check in your patch so users
wouldn't have to switch the Windows version in winecfg. We still need
the separate thread implementation.

If I were to tackle this, I would reproduce the exact sequence of
steps that the program takes, with identical parameters and structs,
and see what happens when run on Windows. Then try to figure out which
step or piece of data triggers the behavior switch.

Andrew

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Bruno Jesus]

On Thu, Dec 1, 2016 at 4:44 PM, Andrew Eikum  wrote:
> > I don't know what else to do, you can read some more at
> > https://bugs.winehq.org/show_bug.cgi?id=3930#c18
>
> Huh, interesting. Either Windows has magic for this game, or it uses
> some other heuristic to determine whether to use the old behavior.
> > It would not be the first time Windows circumvents games problems. My
> > patch is only touching Wine as configured for 9x, it brings no issues
> > to the general user and makes Wine behave as 9x did. I cannot solve
> > the mistery of how XP detects the game and avoids SuspendThread as my
> > knowledge is limited and I believe it would require reverse
> > engineering.
>
> To be clear, I'm not rejecting your idea. Whatever this heuristic is,
> it would just replace the Windows version check in your patch so users
> wouldn't have to switch the Windows version in winecfg. We still need
> the separate thread implementation.

Thanks, re-reading what I wrote may have sounded like I'm angry or
something like that but it is just my simple English vocabulary. In a
different point of view I could say that since the next stable is
getting closer this solution would really affect the minimum amount of
people possible, and that I don't have the skills to debug anything as
my assembly knowledge is limited to mov, add, xor, cmp, jmp, jnz ;-)

If we can bypass SuspendThread there wouldn't be a need for the
thread, a hack that works for this bug is to comment out
Suspend/ResumeThread in Wine code. But I guess it is harder than
creating the thread since there is no simple way to do that in Windows
API AFAICS.
> If I were to tackle this, I would reproduce the exact sequence of
> steps that the program takes, with identical parameters and structs,
> and see what happens when run on Windows. Then try to figure out which
> step or piece of data triggers the behavior switch.

I'll see what I can do by following the relay logs.

Best wishes,
Bruno

[PATCH 2/2] winmm: Use a thread to send driver messages on Win9x (try 2)

[Andrew Eikum]

On Thu, Dec 01, 2016 at 04:58:37PM -0200, Bruno Jesus wrote:
> On Thu, Dec 1, 2016 at 4:44 PM, Andrew Eikum  wrote:
> > If I were to tackle this, I would reproduce the exact sequence of
> > steps that the program takes, with identical parameters and structs,
> > and see what happens when run on Windows. Then try to figure out which
> > step or piece of data triggers the behavior switch.
>
> I'll see what I can do by following the relay logs.

Well, I meant specifically how it interacts with winmm. I think it's
more likely that native winmm has some trigger to do the win9x
threading behavior than it kills SuspendThread. But anything's
possible.

In any case, it might be best to wait until after the next release.
It's easy to have unintended consequences in winmm.

Andrew