Re: [PATCH] winmm: Use a thread to send driver messages on Win9x

[Nikolay Sivov]

On Thu,  1 Dec 2016 04:40:22 -0200
Bruno Jesus <00cpxxx(a)gmail.com> wrote:
> 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 <00cpxxx(a)gmail.com>
> ---
>  dlls/winmm/waveform.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/winmm/waveform.c b/dlls/winmm/waveform.c
> index 7308519..24d7e30 100644
> --- a/dlls/winmm/waveform.c
> +++ b/dlls/winmm/waveform.c
> @@ -134,6 +134,14 @@ struct _WINMM_MMDevice {
>      WINMM_Device *devices[MAX_DEVICES];
>  };
>
> +static struct {
> +    BOOL is, checked;
> +    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;
> @@ -286,6 +294,13 @@ static WINMM_Device *WINMM_FindUnusedDevice(WINMM_Device **devices,
>  {
>      UINT i;
>
> +    if (!win9x.checked)
> +    {
> +        win9x.checked = TRUE;
> +        if ((GetVersion() & 0xFF) < 5)
> +            win9x.is = TRUE;
> +    }
> +

This only needs to happen during dll initialization. I think cleaner way for a change you're proposing
will be two separate vtable structures, one for win9x and another one for the rest.
This way you can select appropriate vtable once on dll initialization, and keep this selection forever
as a global pointer for example.
> /* 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);
> +    if (win9x.is)
> +    {
> HeapFree(GetProcessHeap(), 0, device->orig_fmt);
> +    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;
> +    }
>
> return S_OK;
>  }

This will also let you to get rid of conditional implementation like above.