[PATCH v2 1/5] dinput: Do not wait for internal hooks inside the HID joystick CS.

[Rémi Bernon]

This can cause some deadlocks as the internal thread will acquire the
device CS when reading the state.

Signed-off-by: Rémi Bernon 
---

v2: * Set acquired flag before calling SendForceFeedbackCommand.

* Add missing HRESULT hr declaration, lost in a split.

Supersedes: 216794-216798

dlls/dinput/joystick_hid.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c
index 75bf66b7a51..94caf59506e 100644
--- a/dlls/dinput/joystick_hid.c
+++ b/dlls/dinput/joystick_hid.c
@@ -881,22 +881,32 @@ static HRESULT WINAPI hid_joystick_Acquire( IDirectInputDevice8W *iface )
 {
     struct hid_joystick *impl = impl_from_IDirectInputDevice8W( iface );
     ULONG report_len = impl->caps.InputReportByteLength;
-    HRESULT hr;
+    HRESULT hr = DI_OK;

TRACE( "iface %p.\n", iface );

EnterCriticalSection( &impl->base.crit );
-    hr = IDirectInputDevice2WImpl_Acquire( iface );
-    if (hr == DI_OK)
+    if (impl->base.acquired)
+        hr = DI_NOEFFECT;
+    else if (!impl->base.data_format.user_df)
+        hr = DIERR_INVALIDPARAM;
+    else if ((impl->base.dwCoopLevel & DISCL_FOREGROUND) && impl->base.win != GetForegroundWindow())
+        hr = DIERR_OTHERAPPHASPRIO;
+    else
     {
         memset( &impl->read_ovl, 0, sizeof(impl->read_ovl) );
         impl->read_ovl.hEvent = impl->base.read_event;
         if (ReadFile( impl->device, impl->input_report_buf, report_len, NULL, &impl->read_ovl ))
             impl->base.read_callback( iface );

+        impl->base.acquired = TRUE;
         IDirectInputDevice8_SendForceFeedbackCommand( iface, DISFFC_RESET );
     }
     LeaveCriticalSection( &impl->base.crit );
+    if (hr != DI_OK) return hr;
+
+    dinput_hooks_acquire_device( iface );
+    check_dinput_hooks( iface, TRUE );

return hr;
 }
@@ -904,21 +914,26 @@ static HRESULT WINAPI hid_joystick_Acquire( IDirectInputDevice8W *iface )
 static HRESULT WINAPI hid_joystick_Unacquire( IDirectInputDevice8W *iface )
 {
     struct hid_joystick *impl = impl_from_IDirectInputDevice8W( iface );
-    HRESULT hr;
+    HRESULT hr = DI_OK;
     BOOL ret;

TRACE( "iface %p.\n", iface );

EnterCriticalSection( &impl->base.crit );
-    if (impl->base.acquired)
+    if (!impl->base.acquired) hr = DI_NOEFFECT;
+    else
     {
         ret = CancelIoEx( impl->device, &impl->read_ovl );
         if (!ret) WARN( "CancelIoEx failed, last error %u\n", GetLastError() );

IDirectInputDevice8_SendForceFeedbackCommand( iface, DISFFC_RESET );
+        impl->base.acquired = FALSE;
     }
-    hr = IDirectInputDevice2WImpl_Unacquire( iface );
     LeaveCriticalSection( &impl->base.crit );
+    if (hr != DI_OK) return hr;
+
+    dinput_hooks_unacquire_device( iface );
+    check_dinput_hooks( iface, FALSE );

return hr;
 }

[PATCH v2 2/5] dinput: Hold the HID joystick CS when updating the device state.

[Rémi Bernon]

The read buffer is only used by the reading thread but the device state
is not, we should only update it while holding the CS.

Signed-off-by: Rémi Bernon 
---
 dlls/dinput/joystick_hid.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c
index 94caf59506e..35d03fbf7cc 100644
--- a/dlls/dinput/joystick_hid.c
+++ b/dlls/dinput/joystick_hid.c
@@ -1495,6 +1495,7 @@ static HRESULT hid_joystick_read_state( IDirectInputDevice8W *iface )
         }
     }

+    EnterCriticalSection( &impl->base.crit );
     do
     {
         count = impl->usages_count;
@@ -1530,6 +1531,7 @@ static HRESULT hid_joystick_read_state( IDirectInputDevice8W *iface )
         memset( &impl->read_ovl, 0, sizeof(impl->read_ovl) );
         impl->read_ovl.hEvent = impl->base.read_event;
     } while (ReadFile( impl->device, report_buf, report_len, &count, &impl->read_ovl ));
+    LeaveCriticalSection( &impl->base.crit );

return DI_OK;
 }

[PATCH v2 3/5] dinput: Wait for CancelIoEx completion when unacquiring HID joysticks.

[Rémi Bernon]

Otherwise we may later write the cancelled status to invalid memory.

Also use a manual-reset event, as it should be for overlapped I/O, so
all waiters are woken up on cancel.

Signed-off-by: Rémi Bernon 
---
 dlls/dinput/joystick_hid.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c
index 35d03fbf7cc..0453cd7d529 100644
--- a/dlls/dinput/joystick_hid.c
+++ b/dlls/dinput/joystick_hid.c
@@ -543,7 +543,6 @@ static ULONG hid_joystick_private_decref( struct hid_joystick *impl )
         HeapFree( GetProcessHeap(), 0, tmp.input_report_buf );
         HeapFree( GetProcessHeap(), 0, tmp.input_extra_caps );
         HidD_FreePreparsedData( tmp.preparsed );
-        CancelIoEx( tmp.device, &tmp.read_ovl );
         CloseHandle( tmp.base.read_event );
         CloseHandle( tmp.device );
     }
@@ -925,7 +924,7 @@ static HRESULT WINAPI hid_joystick_Unacquire( IDirectInputDevice8W *iface )
     {
         ret = CancelIoEx( impl->device, &impl->read_ovl );
         if (!ret) WARN( "CancelIoEx failed, last error %u\n", GetLastError() );
-
+        else WaitForSingleObject( impl->base.read_event, INFINITE );
         IDirectInputDevice8_SendForceFeedbackCommand( iface, DISFFC_RESET );
         impl->base.acquired = FALSE;
     }
@@ -2009,7 +2008,7 @@ static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, const GUID
     if (FAILED(hr)) return hr;
     impl->base.crit.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": hid_joystick.base.crit");
     impl->base.dwCoopLevel = DISCL_NONEXCLUSIVE | DISCL_BACKGROUND;
-    impl->base.read_event = CreateEventA( NULL, FALSE, FALSE, NULL );
+    impl->base.read_event = CreateEventW( NULL, TRUE, FALSE, NULL );
     impl->base.read_callback = hid_joystick_read_state;

hr = hid_joystick_device_open( -1, &instance, impl->device_path, &impl->device, &impl->preparsed,

[PATCH v2 4/5] dinput: Call device read_callback while holding the internal CS.

[Rémi Bernon]

And remove the device from the list of acquired devices if the callback
indicates a read error.

Signed-off-by: Rémi Bernon 
---
 dlls/dinput/dinput_main.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/dlls/dinput/dinput_main.c b/dlls/dinput/dinput_main.c
index 1cd6d390430..9c0e3875c00 100644
--- a/dlls/dinput/dinput_main.c
+++ b/dlls/dinput/dinput_main.c
@@ -1289,11 +1289,11 @@ static LRESULT CALLBACK callwndproc_proc( int code, WPARAM wparam, LPARAM lparam
 static DWORD WINAPI hook_thread_proc(void *param)
 {
     static HHOOK kbd_hook, mouse_hook;
-    IDirectInputDeviceImpl *impl;
-    IDirectInputDevice8W *iface;
+    IDirectInputDeviceImpl *impl, *next;
     SIZE_T events_count = 0;
     HANDLE finished_event;
     HANDLE events[128];
+    HRESULT hr;
     DWORD ret;
     MSG msg;

@@ -1310,24 +1310,17 @@ static DWORD WINAPI hook_thread_proc(void *param)

if (ret < events_count)
         {
-            iface = NULL;
             EnterCriticalSection( &dinput_hook_crit );
-            LIST_FOR_EACH_ENTRY( impl, &acquired_device_list, IDirectInputDeviceImpl, entry )
+            LIST_FOR_EACH_ENTRY_SAFE( impl, next, &acquired_device_list, IDirectInputDeviceImpl, entry )
             {
                 if (impl->read_event == events[ret])
                 {
-                    iface = &impl->IDirectInputDevice8W_iface;
-                    IDirectInputDevice8_AddRef( iface );
+                    hr = impl->read_callback( &impl->IDirectInputDevice8W_iface );
+                    if (FAILED(hr)) list_remove( &impl->entry );
                     break;
                 }
             }
             LeaveCriticalSection( &dinput_hook_crit );
-
-            if (iface)
-            {
-                impl->read_callback( iface );
-                IDirectInputDevice8_Release( iface );
-            }
         }

while (PeekMessageW( &msg, 0, 0, 0, PM_REMOVE ))

[PATCH v2 5/5] dinput: Check for ReadFile errors and return DIERR_INPUTLOST.

[Rémi Bernon]

Signed-off-by: Rémi Bernon 
---
 dlls/dinput/joystick_hid.c | 57 ++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 14 deletions(-)

diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c
index 0453cd7d529..837a8b4aab9 100644
--- a/dlls/dinput/joystick_hid.c
+++ b/dlls/dinput/joystick_hid.c
@@ -881,6 +881,7 @@ static HRESULT WINAPI hid_joystick_Acquire( IDirectInputDevice8W *iface )
     struct hid_joystick *impl = impl_from_IDirectInputDevice8W( iface );
     ULONG report_len = impl->caps.InputReportByteLength;
     HRESULT hr = DI_OK;
+    BOOL ret;

TRACE( "iface %p.\n", iface );

@@ -891,15 +892,29 @@ static HRESULT WINAPI hid_joystick_Acquire( IDirectInputDevice8W *iface )
         hr = DIERR_INVALIDPARAM;
     else if ((impl->base.dwCoopLevel & DISCL_FOREGROUND) && impl->base.win != GetForegroundWindow())
         hr = DIERR_OTHERAPPHASPRIO;
-    else
+    else if (impl->device == INVALID_HANDLE_VALUE)
+    {
+        impl->device = CreateFileW( impl->device_path, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE,
+                                    NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED | FILE_FLAG_NO_BUFFERING, 0 );
+        if (impl->device == INVALID_HANDLE_VALUE) hr = DIERR_INPUTLOST;
+    }
+
+    if (hr == DI_OK)
     {
         memset( &impl->read_ovl, 0, sizeof(impl->read_ovl) );
         impl->read_ovl.hEvent = impl->base.read_event;
-        if (ReadFile( impl->device, impl->input_report_buf, report_len, NULL, &impl->read_ovl ))
-            impl->base.read_callback( iface );
-
-        impl->base.acquired = TRUE;
-        IDirectInputDevice8_SendForceFeedbackCommand( iface, DISFFC_RESET );
+        ret = ReadFile( impl->device, impl->input_report_buf, report_len, NULL, &impl->read_ovl );
+        if (!ret && GetLastError() != ERROR_IO_PENDING)
+        {
+            CloseHandle( impl->device );
+            impl->device = INVALID_HANDLE_VALUE;
+            hr = DIERR_INPUTLOST;
+        }
+        else
+        {
+            impl->base.acquired = TRUE;
+            IDirectInputDevice8_SendForceFeedbackCommand( iface, DISFFC_RESET );
+        }
     }
     LeaveCriticalSection( &impl->base.crit );
     if (hr != DI_OK) return hr;
@@ -922,9 +937,12 @@ static HRESULT WINAPI hid_joystick_Unacquire( IDirectInputDevice8W *iface )
     if (!impl->base.acquired) hr = DI_NOEFFECT;
     else
     {
-        ret = CancelIoEx( impl->device, &impl->read_ovl );
-        if (!ret) WARN( "CancelIoEx failed, last error %u\n", GetLastError() );
-        else WaitForSingleObject( impl->base.read_event, INFINITE );
+        if (impl->device != INVALID_HANDLE_VALUE)
+        {
+            ret = CancelIoEx( impl->device, &impl->read_ovl );
+            if (!ret) WARN( "CancelIoEx failed, last error %u\n", GetLastError() );
+            else WaitForSingleObject( impl->base.read_event, INFINITE );
+        }
         IDirectInputDevice8_SendForceFeedbackCommand( iface, DISFFC_RESET );
         impl->base.acquired = FALSE;
     }
@@ -1475,11 +1493,11 @@ static HRESULT hid_joystick_read_state( IDirectInputDevice8W *iface )
     char *report_buf = impl->input_report_buf;
     USAGE_AND_PAGE *usages;
     NTSTATUS status;
+    HRESULT hr;
     BOOL ret;

ret = GetOverlappedResult( impl->device, &impl->read_ovl, &count, FALSE );
-    if (!ret) WARN( "ReadFile failed, error %u\n", GetLastError() );
-    else if (TRACE_ON(dinput))
+    if (ret && TRACE_ON(dinput))
     {
         TRACE( "read size %u report:\n", count );
         for (i = 0; i < report_len;)
@@ -1495,7 +1513,7 @@ static HRESULT hid_joystick_read_state( IDirectInputDevice8W *iface )
     }

EnterCriticalSection( &impl->base.crit );
-    do
+    while (ret)
     {
         count = impl->usages_count;
         memset( impl->usages_buf, 0, count * sizeof(*impl->usages_buf) );
@@ -1529,10 +1547,21 @@ static HRESULT hid_joystick_read_state( IDirectInputDevice8W *iface )

memset( &impl->read_ovl, 0, sizeof(impl->read_ovl) );
         impl->read_ovl.hEvent = impl->base.read_event;
-    } while (ReadFile( impl->device, report_buf, report_len, &count, &impl->read_ovl ));
+        ret = ReadFile( impl->device, report_buf, report_len, &count, &impl->read_ovl );
+    }
+
+    if (GetLastError() == ERROR_IO_PENDING || GetLastError() == ERROR_OPERATION_ABORTED) hr = DI_OK;
+    else
+    {
+        WARN( "GetOverlappedResult/ReadFile failed, error %u\n", GetLastError() );
+        CloseHandle(impl->device);
+        impl->device = INVALID_HANDLE_VALUE;
+        impl->base.acquired = FALSE;
+        hr = DIERR_INPUTLOST;
+    }
     LeaveCriticalSection( &impl->base.crit );

-    return DI_OK;
+    return hr;
 }

static DWORD device_type_for_version( DWORD type, DWORD version )