[PATCH v2] dinput: Wait for the internal thread to complete unacquiring.

[Rémi Bernon]

Otherwise we may close HID read event before it has finished removing it
from the wait handles, causing MsgWaitForMultipleObjectsEx to return an
error and terminate.

Signed-off-by: Rémi Bernon 
---
 dlls/dinput/dinput_main.c | 13 ++++---------
 dlls/dinput8/tests/hid.c  |  6 ------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/dlls/dinput/dinput_main.c b/dlls/dinput/dinput_main.c
index b385cc7503c..61cecb34b16 100644
--- a/dlls/dinput/dinput_main.c
+++ b/dlls/dinput/dinput_main.c
@@ -1364,8 +1364,7 @@ static DWORD WINAPI hook_thread_proc(void *param)
                 mouse_hook = NULL;
             }

-            if (finished_event)
-                SetEvent(finished_event);
+            SetEvent(finished_event);
         }
     }

@@ -1487,17 +1486,13 @@ void check_dinput_hooks(LPDIRECTINPUTDEVICE8W iface, BOOL acquired)
             WARN( "Unable to (un)register raw device %x:%x\n", dev->raw_device.usUsagePage, dev->raw_device.usUsage );
     }

-    if (acquired)
-        hook_change_finished_event = CreateEventW( NULL, FALSE, FALSE, NULL );
+    hook_change_finished_event = CreateEventW( NULL, FALSE, FALSE, NULL );
     PostThreadMessageW( hook_thread_id, WM_USER+0x10, 1, (LPARAM)hook_change_finished_event );

LeaveCriticalSection(&dinput_hook_crit);

-    if (acquired)
-    {
-        WaitForSingleObject(hook_change_finished_event, INFINITE);
-        CloseHandle(hook_change_finished_event);
-    }
+    WaitForSingleObject(hook_change_finished_event, INFINITE);
+    CloseHandle(hook_change_finished_event);
 }

void check_dinput_events(void)
diff --git a/dlls/dinput8/tests/hid.c b/dlls/dinput8/tests/hid.c
index a2719f65488..6408fd0c1f0 100644
--- a/dlls/dinput8/tests/hid.c
+++ b/dlls/dinput8/tests/hid.c
@@ -4759,9 +4759,6 @@ static void test_simple_joystick(void)
     todo_wine
     ok( hr == DIERR_UNSUPPORTED, "Escape returned: %#x\n", hr );

-    /* FIXME: we have to wait a bit because Wine DInput internal thread keeps a reference */
-    Sleep( 100 );
-
     ref = IDirectInputDevice8_Release( device );
     ok( ref == 0, "Release returned %d\n", ref );

@@ -7228,9 +7225,6 @@ static void test_force_feedback_joystick( void )
     ok( hr == DI_OK, "Unacquire returned: %#x\n", hr );
     set_hid_expect( file, NULL, 0 );

-    /* FIXME: we have to wait a bit because Wine DInput internal thread keeps a reference */
-    Sleep( 100 );
-
     ref = IDirectInputDevice8_Release( device );
     ok( ref == 0, "Release returned %d\n", ref );

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=99875

Your paranoid android.

=== debiant2 (32 bit report) ===

dinput:
mouse: Timeout

=== debiant2 (32 bit Chinese:China report) ===

dinput:
mouse: Timeout

=== debiant2 (32 bit WoW report) ===

dinput:
mouse: Timeout

=== debiant2 (64 bit WoW report) ===

dinput:
mouse: Timeout

[Rémi Bernon]

On 10/12/21 1:11 PM, Marvin wrote:
> Hi,
>
> While running your changed tests, I think I found new failures.
> Being a bot and all I'm not very good at pattern recognition, so I might be
> wrong, but could you please double-check?
>
> Full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=99875
>
> Your paranoid android.
>
> === debiant2 (32 bit report) ===
>
> dinput:
> mouse: Timeout
>
> === debiant2 (32 bit Chinese:China report) ===
>
> dinput:
> mouse: Timeout
>
> === debiant2 (32 bit WoW report) ===
>
> dinput:
> mouse: Timeout
>
> === debiant2 (64 bit WoW report) ===
>
> dinput:
> mouse: Timeout

I was kind of expecting something like this. I'll investigate.