[PATCH 0/5] MR7997: windows.device.enumeration: Implement initial device enumeration for DeviceWatcher.
After `DeviceWatcher::Start()` gets called, `DeviceWatcher` enumerates through a list of all `DeviceInformation` objects on the current system through the `Added` event handlers, dispatching `EnumerationCompleted` once this has finished. This MR implements this functionality. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/windows.devices.enumeration/main.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index ba7edaa5199..b1890235c6a 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -31,6 +31,7 @@ struct device_watcher IDeviceWatcher IDeviceWatcher_iface; LONG ref; + struct list added_handlers; struct list stopped_handlers; HSTRING filter; }; @@ -76,6 +77,7 @@ static ULONG WINAPI device_watcher_Release( IDeviceWatcher *iface ) if (!ref) { + typed_event_handlers_clear( &impl->added_handlers ); typed_event_handlers_clear( &impl->stopped_handlers ); WindowsDeleteString( impl->filter ); free( impl ); @@ -105,14 +107,18 @@ static HRESULT WINAPI device_watcher_GetTrustLevel( IDeviceWatcher *iface, Trust static HRESULT WINAPI device_watcher_add_Added( IDeviceWatcher *iface, ITypedEventHandler_DeviceWatcher_DeviceInformation *handler, EventRegistrationToken *token ) { - FIXME( "iface %p, handler %p, token %p stub!\n", iface, handler, token ); - return S_OK; + struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); + + TRACE( "iface %p, handler %p, token %p\n", iface, handler, token ); + return typed_event_handlers_append( &impl->added_handlers, (ITypedEventHandler_IInspectable_IInspectable *)handler, token ); } static HRESULT WINAPI device_watcher_remove_Added( IDeviceWatcher *iface, EventRegistrationToken token ) { - FIXME( "iface %p, token %#I64x stub!\n", iface, token.value ); - return E_NOTIMPL; + struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); + + TRACE( "iface %p, token %#I64x.\n", iface, token.value ); + return typed_event_handlers_remove( &impl->added_handlers, &token ); } static HRESULT WINAPI device_watcher_add_Updated( IDeviceWatcher *iface, ITypedEventHandler_DeviceWatcher_DeviceInformationUpdate *handler, @@ -465,6 +471,7 @@ static HRESULT WINAPI device_statics_CreateWatcherAqsFilter( IDeviceInformationS return hr; } + list_init( &this->added_handlers ); list_init( &this->stopped_handlers ); *watcher = &this->IDeviceWatcher_iface; @@ -548,6 +555,7 @@ static HRESULT WINAPI device_statics2_CreateWatcher( IDeviceInformationStatics2 return hr; } + list_init( &this->added_handlers ); list_init( &this->stopped_handlers ); *watcher = &this->IDeviceWatcher_iface; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7997
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/windows.devices.enumeration/main.c | 4 ++-- dlls/windows.devices.enumeration/tests/devices.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index b1890235c6a..de6ae50fc0d 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -444,8 +444,8 @@ static HRESULT WINAPI device_statics_FindAllAsyncAqsFilterAndAdditionalPropertie static HRESULT WINAPI device_statics_CreateWatcher( IDeviceInformationStatics *iface, IDeviceWatcher **watcher ) { - FIXME( "iface %p, watcher %p stub!\n", iface, watcher ); - return E_NOTIMPL; + TRACE( "iface %p, watcher %p\n", iface, watcher ); + return IDeviceInformationStatics_CreateWatcherAqsFilter( iface, NULL, watcher ); } static HRESULT WINAPI device_statics_CreateWatcherDeviceClass( IDeviceInformationStatics *iface, DeviceClass class, IDeviceWatcher **watcher ) diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index 730237fcd84..2c82c139d7f 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -465,7 +465,7 @@ static void test_DeviceInformation( void ) IDeviceWatcher_Release( device_watcher ); hr = IDeviceInformationStatics_CreateWatcher( device_info_statics, &device_watcher ); - todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); + ok( hr == S_OK, "got hr %#lx\n", hr ); if (device_watcher) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7997
From: Vibhav Pant <vibhavp(a)gmail.com> --- .../tests/devices.c | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index a6c0ce0fe03..730237fcd84 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -58,6 +58,7 @@ struct device_watcher_handler ITypedEventHandler_DeviceWatcher_IInspectable ITypedEventHandler_DeviceWatcher_IInspectable_iface; LONG ref; + unsigned int test_deviceinformation : 1; HANDLE event; BOOL invoked; IInspectable *args; @@ -101,6 +102,7 @@ static ULONG WINAPI device_watcher_handler_Release( ITypedEventHandler_DeviceWat return ref; } +static void test_DeviceInformation_obj( int line, IDeviceInformation *info ); static HRESULT WINAPI device_watcher_handler_Invoke( ITypedEventHandler_DeviceWatcher_IInspectable *iface, IDeviceWatcher *sender, IInspectable *args ) { @@ -115,6 +117,17 @@ static HRESULT WINAPI device_watcher_handler_Invoke( ITypedEventHandler_DeviceWa ref = IDeviceWatcher_Release( sender ); ok( ref == 3, "got ref %lu\n", ref ); + if (impl->test_deviceinformation) + { + IDeviceInformation *info; + HRESULT hr; + + hr = IInspectable_QueryInterface( args, &IID_IDeviceInformation, (void *)&info ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + test_DeviceInformation_obj( __LINE__, info ); + IDeviceInformation_Release( info ); + } + SetEvent( impl->event ); return S_OK; @@ -318,8 +331,8 @@ static void test_DeviceInformation( void ) { static const WCHAR *device_info_name = L"Windows.Devices.Enumeration.DeviceInformation"; - static struct device_watcher_handler stopped_handler, added_handler; - EventRegistrationToken stopped_token, added_token; + static struct device_watcher_handler stopped_handler, added_handler, enumerated_handler; + EventRegistrationToken stopped_token, added_token, enumerated_token; IInspectable *inspectable, *inspectable2; IActivationFactory *factory; IDeviceInformationStatics2 *device_info_statics2; @@ -336,8 +349,12 @@ static void test_DeviceInformation( void ) device_watcher_handler_create( &added_handler ); device_watcher_handler_create( &stopped_handler ); + device_watcher_handler_create( &enumerated_handler ); + stopped_handler.event = CreateEventW( NULL, FALSE, FALSE, NULL ); ok( !!stopped_handler.event, "failed to create event, got error %lu\n", GetLastError() ); + enumerated_handler.event = CreateEventW( NULL, FALSE, FALSE, NULL ); + ok( !!enumerated_handler.event, "failed to create event, got error %lu\n", GetLastError() ); hr = WindowsCreateString( device_info_name, wcslen( device_info_name ), &str ); ok( hr == S_OK, "got hr %#lx\n", hr ); @@ -410,7 +427,7 @@ static void test_DeviceInformation( void ) goto skip_device_statics; } - IDeviceInformationStatics_CreateWatcherAqsFilter( device_info_statics, NULL, &device_watcher ); + hr = IDeviceInformationStatics_CreateWatcherAqsFilter( device_info_statics, NULL, &device_watcher ); ok( hr == S_OK, "got hr %#lx\n", hr ); check_interface( device_watcher, &IID_IUnknown, TRUE ); @@ -447,6 +464,33 @@ static void test_DeviceInformation( void ) IDeviceWatcher_Release( device_watcher ); + hr = IDeviceInformationStatics_CreateWatcher( device_info_statics, &device_watcher ); + todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); + + if (device_watcher) + { + added_handler.test_deviceinformation = 1; + hr = IDeviceWatcher_add_Added( device_watcher, (void *)&added_handler.ITypedEventHandler_DeviceWatcher_IInspectable_iface, &added_token ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + hr = IDeviceWatcher_add_EnumerationCompleted( device_watcher, (void *)&enumerated_handler.ITypedEventHandler_DeviceWatcher_IInspectable_iface, &enumerated_token ); + todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); + + ref = IDeviceWatcher_AddRef( device_watcher ); + ok( ref == 2, "got ref %lu\n", ref ); + hr = IDeviceWatcher_Start( device_watcher ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine ok( !WaitForSingleObject( enumerated_handler.event, 5000 ), "wait for enumerated_handler.event failed\n" ); + hr = IDeviceWatcher_get_Status( device_watcher, &status ); + todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine ok( status == DeviceWatcherStatus_EnumerationCompleted, "got status %u\n", status ); + + hr = IDeviceWatcher_Start( device_watcher ); + todo_wine ok( hr == E_ILLEGAL_METHOD_CALL, "Start returned %#lx\n", hr ); + + IDeviceWatcher_Release( device_watcher ); + IDeviceWatcher_Release( device_watcher ); + } + hr = IDeviceInformationStatics_FindAllAsync( device_info_statics, &info_collection_async ); ok( hr == S_OK, "got %#lx\n", hr ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7997
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/windows.devices.enumeration/main.c | 218 ++++++++++++++---- .../tests/devices.c | 4 +- 2 files changed, 179 insertions(+), 43 deletions(-) diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index 78d77effb35..aee89d9442d 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -26,17 +26,54 @@ WINE_DEFAULT_DEBUG_CHANNEL(enumeration); +struct device_watcher_weak; struct device_watcher { IDeviceWatcher IDeviceWatcher_iface; - LONG ref; + struct device_watcher_weak *weakref; struct list added_handlers; struct list enumerated_handlers; struct list stopped_handlers; HSTRING filter; + + CRITICAL_SECTION cs; + DeviceWatcherStatus status; }; +struct device_watcher_weak +{ + LONG weak_count; + LONG strong_count; + struct device_watcher *watcher; +}; + +static struct device_watcher *device_watcher_weak_resolve( struct device_watcher_weak *weak ) +{ + LONG count = weak->strong_count; + + while (count) + { + if (InterlockedCompareExchange( &weak->strong_count, count + 1, count) == count) + return weak->watcher; + + count = weak->strong_count; + } + return NULL; +} + +static void device_watcher_weak_decref( struct device_watcher_weak *weak ) +{ + if (!InterlockedDecrement( &weak->weak_count )) + free( weak ); +} + +static struct device_watcher_weak *device_watcher_make_weak( struct device_watcher *watcher ) +{ + InterlockedIncrement( &watcher->weakref->weak_count ); + return watcher->weakref; +} + static inline struct device_watcher *impl_from_IDeviceWatcher( IDeviceWatcher *iface ) { return CONTAINING_RECORD( iface, struct device_watcher, IDeviceWatcher_iface ); @@ -65,7 +102,7 @@ static HRESULT WINAPI device_watcher_QueryInterface( IDeviceWatcher *iface, REFI static ULONG WINAPI device_watcher_AddRef( IDeviceWatcher *iface ) { struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); - ULONG ref = InterlockedIncrement( &impl->ref ); + ULONG ref = InterlockedIncrement( &impl->weakref->strong_count ); TRACE( "iface %p, ref %lu.\n", iface, ref ); return ref; } @@ -73,7 +110,7 @@ static ULONG WINAPI device_watcher_AddRef( IDeviceWatcher *iface ) static ULONG WINAPI device_watcher_Release( IDeviceWatcher *iface ) { struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); - ULONG ref = InterlockedDecrement( &impl->ref ); + ULONG ref = InterlockedDecrement( &impl->weakref->strong_count ); TRACE( "iface %p, ref %lu.\n", iface, ref ); if (!ref) @@ -82,6 +119,9 @@ static ULONG WINAPI device_watcher_Release( IDeviceWatcher *iface ) typed_event_handlers_clear( &impl->enumerated_handlers ); typed_event_handlers_clear( &impl->stopped_handlers ); WindowsDeleteString( impl->filter ); + device_watcher_weak_decref( impl->weakref ); + impl->cs.DebugInfo->Spare[0] = 0; + DeleteCriticalSection( &impl->cs ); free( impl ); } @@ -188,9 +228,107 @@ static HRESULT WINAPI device_watcher_Status( IDeviceWatcher *iface, DeviceWatche return E_NOTIMPL; } +static HRESULT WINAPI find_all_async( IUnknown *invoker, IUnknown *param, PROPVARIANT *result ); + +static void CALLBACK device_watcher_enumerate_proc( TP_CALLBACK_INSTANCE *instance, void *watcher_weak ) +{ + IVectorView_DeviceInformation *collection = NULL; + struct device_watcher *impl; + IDeviceInformation *info; + IDeviceWatcher *watcher; + PROPVARIANT result; + UINT32 i, size; + HRESULT hr; + + if (!(impl = device_watcher_weak_resolve( watcher_weak ))) + { + device_watcher_weak_decref( watcher_weak ); + return; + } + watcher = &impl->IDeviceWatcher_iface; + + PropVariantInit( &result ); + if (FAILED( hr = find_all_async( NULL, NULL, &result ) )) + goto done; + + collection = (IVectorView_DeviceInformation *)result.punkVal; + if (!WindowsIsStringEmpty( impl->filter )) + { + FIXME( "Unsupported filter: %s\n", debugstr_hstring( impl->filter ) ); + goto done; + } + if (FAILED( hr = IVectorView_DeviceInformation_get_Size( collection, &size ))) + goto done; + for (i = 0; i < size && SUCCEEDED( hr = IVectorView_DeviceInformation_GetAt( collection, i, &info ) ); i++) + { + DeviceWatcherStatus status; + + EnterCriticalSection( &impl->cs ); + status = impl->status; + LeaveCriticalSection( &impl->cs ); + if (status != DeviceWatcherStatus_Started) + { + IDeviceInformation_Release( info ); + goto done; + } + + typed_event_handlers_notify( &impl->added_handlers, (IInspectable *)watcher, (IInspectable *)info ); + IDeviceInformation_Release( info ); + } + +done: + EnterCriticalSection( &impl->cs ); + if (impl->status == DeviceWatcherStatus_Started) + { + impl->status = DeviceWatcherStatus_EnumerationCompleted; + LeaveCriticalSection( &impl->cs ); + typed_event_handlers_notify( &impl->enumerated_handlers, (IInspectable *)watcher, NULL ); + } + else + LeaveCriticalSection( &impl->cs ); + + if (collection) + IVectorView_DeviceInformation_Release( collection ); + IDeviceWatcher_Release( watcher ); + device_watcher_weak_decref( watcher_weak ); +} + static HRESULT WINAPI device_watcher_Start( IDeviceWatcher *iface ) { - FIXME( "iface %p stub!\n", iface ); + struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); + + FIXME( "iface %p: semi-stub!\n", iface ); + + EnterCriticalSection( &impl->cs ); + switch (impl->status) + { + case DeviceWatcherStatus_Created: + case DeviceWatcherStatus_Stopped: + case DeviceWatcherStatus_Aborted: + { + struct device_watcher_weak *weak; + + if (!(weak = device_watcher_make_weak( impl ))) + { + LeaveCriticalSection( &impl->cs ); + return E_OUTOFMEMORY; + } + impl->status = DeviceWatcherStatus_Started; + if (!TrySubmitThreadpoolCallback( device_watcher_enumerate_proc, weak, NULL )) + { + DWORD err = GetLastError(); + LeaveCriticalSection( &impl->cs ); + ERR( "TrySubmitThreadpoolCallback failed: %lu\n", err ); + device_watcher_weak_decref( weak ); + return HRESULT_FROM_WIN32( err ); + } + break; + } + default: + LeaveCriticalSection( &impl->cs ); + return E_ILLEGAL_METHOD_CALL; + } + LeaveCriticalSection( &impl->cs ); return S_OK; } @@ -201,6 +339,10 @@ static HRESULT WINAPI device_watcher_Stop( IDeviceWatcher *iface ) FIXME( "iface %p semi-stub!\n", iface ); + EnterCriticalSection( &impl->cs ); + impl->status = DeviceWatcherStatus_Stopped; + LeaveCriticalSection( &impl->cs ); + IDeviceWatcher_AddRef( &impl->IDeviceWatcher_iface ); hr = typed_event_handlers_notify( &impl->stopped_handlers, (IInspectable *)iface, NULL ); IDeviceWatcher_Release( &impl->IDeviceWatcher_iface ); @@ -448,35 +590,31 @@ static HRESULT WINAPI device_statics_FindAllAsyncAqsFilterAndAdditionalPropertie return E_NOTIMPL; } -static HRESULT WINAPI device_statics_CreateWatcher( IDeviceInformationStatics *iface, IDeviceWatcher **watcher ) -{ - TRACE( "iface %p, watcher %p\n", iface, watcher ); - return IDeviceInformationStatics_CreateWatcherAqsFilter( iface, NULL, watcher ); -} - -static HRESULT WINAPI device_statics_CreateWatcherDeviceClass( IDeviceInformationStatics *iface, DeviceClass class, IDeviceWatcher **watcher ) -{ - FIXME( "iface %p, class %d, watcher %p stub!\n", iface, class, watcher ); - return E_NOTIMPL; -} - -static HRESULT WINAPI device_statics_CreateWatcherAqsFilter( IDeviceInformationStatics *iface, HSTRING filter, IDeviceWatcher **watcher ) +static HRESULT device_watcher_create( HSTRING filter, IDeviceWatcher **watcher ) { struct device_watcher *this; HRESULT hr; - TRACE( "iface %p, filter %s, watcher %p\n", iface, debugstr_hstring(filter), watcher ); - if (!(this = calloc( 1, sizeof(*this) ))) return E_OUTOFMEMORY; + if (!((this->weakref = calloc ( 1, sizeof( *this->weakref ) )))) + { + free( this ); + return E_OUTOFMEMORY; + } + + this->weakref->weak_count = this->weakref->strong_count = 1; + this->weakref->watcher = this; this->IDeviceWatcher_iface.lpVtbl = &device_watcher_vtbl; - this->ref = 1; if (FAILED(hr = WindowsDuplicateString( filter, &this->filter ))) { + free( this->weakref ); free( this ); return hr; } + InitializeCriticalSectionEx( &this->cs, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO ); + this->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": device_watcher.cs"); list_init( &this->added_handlers ); list_init( &this->enumerated_handlers ); list_init( &this->stopped_handlers ); @@ -485,6 +623,24 @@ static HRESULT WINAPI device_statics_CreateWatcherAqsFilter( IDeviceInformationS return S_OK; } +static HRESULT WINAPI device_statics_CreateWatcher( IDeviceInformationStatics *iface, IDeviceWatcher **watcher ) +{ + TRACE( "iface %p, watcher %p\n", iface, watcher ); + return device_watcher_create( NULL, watcher ); +} + +static HRESULT WINAPI device_statics_CreateWatcherDeviceClass( IDeviceInformationStatics *iface, DeviceClass class, IDeviceWatcher **watcher ) +{ + FIXME( "iface %p, class %d, watcher %p stub!\n", iface, class, watcher ); + return E_NOTIMPL; +} + +static HRESULT WINAPI device_statics_CreateWatcherAqsFilter( IDeviceInformationStatics *iface, HSTRING filter, IDeviceWatcher **watcher ) +{ + TRACE( "iface %p, filter %s, watcher %p\n", iface, debugstr_hstring(filter), watcher ); + return device_watcher_create( filter, watcher ); +} + static HRESULT WINAPI device_statics_CreateWatcherAqsFilterAndAdditionalProperties( IDeviceInformationStatics *iface, HSTRING filter, IIterable_HSTRING *additional_properties, IDeviceWatcher **watcher ) { @@ -545,29 +701,9 @@ static HRESULT WINAPI device_statics2_CreateWatcher( IDeviceInformationStatics2 IIterable_HSTRING *additional_properties, DeviceInformationKind kind, IDeviceWatcher **watcher ) { - struct device_watcher *this; - HRESULT hr; - FIXME( "iface %p, filter %s, additional_properties %p, kind %u, watcher %p semi-stub!\n", iface, debugstr_hstring( filter ), additional_properties, kind, watcher ); - - if (!(this = calloc( 1, sizeof(*this) ))) - return E_OUTOFMEMORY; - - this->IDeviceWatcher_iface.lpVtbl = &device_watcher_vtbl; - this->ref = 1; - if (FAILED(hr = WindowsDuplicateString( filter, &this->filter ))) - { - free( this ); - return hr; - } - - list_init( &this->added_handlers ); - list_init( &this->enumerated_handlers ); - list_init( &this->stopped_handlers ); - - *watcher = &this->IDeviceWatcher_iface; - return S_OK; + return device_watcher_create( filter, watcher ); } static const struct IDeviceInformationStatics2Vtbl device_statics2_vtbl = diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index 075ae50ed73..bee609629de 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -479,13 +479,13 @@ static void test_DeviceInformation( void ) ok( ref == 2, "got ref %lu\n", ref ); hr = IDeviceWatcher_Start( device_watcher ); ok( hr == S_OK, "got hr %#lx\n", hr ); - todo_wine ok( !WaitForSingleObject( enumerated_handler.event, 5000 ), "wait for enumerated_handler.event failed\n" ); + ok( !WaitForSingleObject( enumerated_handler.event, 5000 ), "wait for enumerated_handler.event failed\n" ); hr = IDeviceWatcher_get_Status( device_watcher, &status ); todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); todo_wine ok( status == DeviceWatcherStatus_EnumerationCompleted, "got status %u\n", status ); hr = IDeviceWatcher_Start( device_watcher ); - todo_wine ok( hr == E_ILLEGAL_METHOD_CALL, "Start returned %#lx\n", hr ); + ok( hr == E_ILLEGAL_METHOD_CALL, "Start returned %#lx\n", hr ); IDeviceWatcher_Release( device_watcher ); IDeviceWatcher_Release( device_watcher ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7997
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/windows.devices.enumeration/main.c | 16 ++++++++++++---- dlls/windows.devices.enumeration/tests/devices.c | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index de6ae50fc0d..78d77effb35 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -32,6 +32,7 @@ struct device_watcher LONG ref; struct list added_handlers; + struct list enumerated_handlers; struct list stopped_handlers; HSTRING filter; }; @@ -78,6 +79,7 @@ static ULONG WINAPI device_watcher_Release( IDeviceWatcher *iface ) if (!ref) { typed_event_handlers_clear( &impl->added_handlers ); + typed_event_handlers_clear( &impl->enumerated_handlers ); typed_event_handlers_clear( &impl->stopped_handlers ); WindowsDeleteString( impl->filter ); free( impl ); @@ -150,14 +152,18 @@ static HRESULT WINAPI device_watcher_remove_Removed( IDeviceWatcher *iface, Even static HRESULT WINAPI device_watcher_add_EnumerationCompleted( IDeviceWatcher *iface, ITypedEventHandler_DeviceWatcher_IInspectable *handler, EventRegistrationToken *token ) { - FIXME( "iface %p, handler %p, token %p stub!\n", iface, handler, token ); - return E_NOTIMPL; + struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); + + TRACE( "iface %p, handler %p, token %p\n", iface, handler, token ); + return typed_event_handlers_append( &impl->enumerated_handlers, (ITypedEventHandler_IInspectable_IInspectable *)handler, token ); } static HRESULT WINAPI device_watcher_remove_EnumerationCompleted( IDeviceWatcher *iface, EventRegistrationToken token ) { - FIXME( "iface %p, token %#I64x stub!\n", iface, token.value ); - return E_NOTIMPL; + struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); + + TRACE( "iface %p, token %#I64x.\n", iface, token.value ); + return typed_event_handlers_remove( &impl->enumerated_handlers, &token ); } static HRESULT WINAPI device_watcher_add_Stopped( IDeviceWatcher *iface, ITypedEventHandler_DeviceWatcher_IInspectable *handler, EventRegistrationToken *token ) @@ -472,6 +478,7 @@ static HRESULT WINAPI device_statics_CreateWatcherAqsFilter( IDeviceInformationS } list_init( &this->added_handlers ); + list_init( &this->enumerated_handlers ); list_init( &this->stopped_handlers ); *watcher = &this->IDeviceWatcher_iface; @@ -556,6 +563,7 @@ static HRESULT WINAPI device_statics2_CreateWatcher( IDeviceInformationStatics2 } list_init( &this->added_handlers ); + list_init( &this->enumerated_handlers ); list_init( &this->stopped_handlers ); *watcher = &this->IDeviceWatcher_iface; diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index 2c82c139d7f..075ae50ed73 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -473,7 +473,7 @@ static void test_DeviceInformation( void ) hr = IDeviceWatcher_add_Added( device_watcher, (void *)&added_handler.ITypedEventHandler_DeviceWatcher_IInspectable_iface, &added_token ); ok( hr == S_OK, "got hr %#lx\n", hr ); hr = IDeviceWatcher_add_EnumerationCompleted( device_watcher, (void *)&enumerated_handler.ITypedEventHandler_DeviceWatcher_IInspectable_iface, &enumerated_token ); - todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); + ok( hr == S_OK, "got hr %#lx\n", hr ); ref = IDeviceWatcher_AddRef( device_watcher ); ok( ref == 2, "got ref %lu\n", ref ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7997
Jinoh Kang (@iamahuman) commented about dlls/windows.devices.enumeration/main.c:
+ LONG count = weak->strong_count; + + while (count) + { + if (InterlockedCompareExchange( &weak->strong_count, count + 1, count) == count) + return weak->watcher; + + count = weak->strong_count; + } + return NULL; +} + +static void device_watcher_weak_decref( struct device_watcher_weak *weak ) +{ + if (!InterlockedDecrement( &weak->weak_count )) + free( weak ); Is it possible that `impl->weakref` is still holding a pointer to `weak`?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102905
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/tests/devices.c:
device_watcher_handler_create( &added_handler ); device_watcher_handler_create( &stopped_handler ); + device_watcher_handler_create( &enumerated_handler ); + stopped_handler.event = CreateEventW( NULL, FALSE, FALSE, NULL ); ok( !!stopped_handler.event, "failed to create event, got error %lu\n", GetLastError() ); + enumerated_handler.event = CreateEventW( NULL, FALSE, FALSE, NULL ); + ok( !!enumerated_handler.event, "failed to create event, got error %lu\n", GetLastError() );
`enumerated_handler.event` is leaked at the end of the function. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102973
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/main.c:
IDeviceWatcher IDeviceWatcher_iface; LONG ref;
+ struct list added_handlers;
I would argue that these maybe should be implemented after the tests have been added (and test should check that the callbacks have been called). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102974
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/main.c:
WINE_DEFAULT_DEBUG_CHANNEL(enumeration);
+struct device_watcher_weak; struct device_watcher { IDeviceWatcher IDeviceWatcher_iface; - LONG ref; + struct device_watcher_weak *weakref;
Does it truly matter for it to use weak refs? Fwiw, if it did (but I don't think it's the case), we should probably check whether it implements IWeakReference* ifaces and do weak ref through them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102976
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/tests/devices.c:
IDeviceWatcher_Release( device_watcher );
+ hr = IDeviceInformationStatics_CreateWatcher( device_info_statics, &device_watcher ); + todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); + + if (device_watcher) + { + added_handler.test_deviceinformation = 1; + hr = IDeviceWatcher_add_Added( device_watcher, (void *)&added_handler.ITypedEventHandler_DeviceWatcher_IInspectable_iface, &added_token ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + hr = IDeviceWatcher_add_EnumerationCompleted( device_watcher, (void *)&enumerated_handler.ITypedEventHandler_DeviceWatcher_IInspectable_iface, &enumerated_token ); + todo_wine ok( hr == S_OK, "got hr %#lx\n", hr ); + + ref = IDeviceWatcher_AddRef( device_watcher ); + ok( ref == 2, "got ref %lu\n", ref );
What is this checking exactly? If this is meant to show that weak ref are used, I don't think it really does that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102975
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/main.c:
+ EnterCriticalSection( &impl->cs ); + switch (impl->status) + { + case DeviceWatcherStatus_Created: + case DeviceWatcherStatus_Stopped: + case DeviceWatcherStatus_Aborted: + { + struct device_watcher_weak *weak; + + if (!(weak = device_watcher_make_weak( impl ))) + { + LeaveCriticalSection( &impl->cs ); + return E_OUTOFMEMORY; + } + impl->status = DeviceWatcherStatus_Started; + if (!TrySubmitThreadpoolCallback( device_watcher_enumerate_proc, weak, NULL )) How about using an async here instead of using thread pool directly?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102977
On Mon May 12 10:36:04 2025 +0000, Rémi Bernon wrote:
What is this checking exactly? If this is meant to show that weak ref are used, I don't think it really does that. Oops, this should've been after the call to Start. Thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102991
On Mon May 12 10:36:05 2025 +0000, Rémi Bernon wrote:
Does it truly matter for it to use weak refs? Fwiw, if it did (but I don't think it's the case), we should probably check whether it implements IWeakReference* ifaces and do weak ref through them. So, the tests above the ones I added here do check if weak references are being used, which is why I added them in the first place. But yes, using `IWeakReference *` would be better.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102992
On Mon May 12 12:03:31 2025 +0000, Vibhav Pant wrote:
So, the tests above the ones I added here do check if weak references are being used, which is why I added them in the first place. But yes, using `IWeakReference *` would be better. Yes, but still I'm not convinced we need to use weak refs. Sure you can add test that show various internal aspects of the implementation but unless there's a good reason (need to break a ref cycle, some application actually depends on returned refcount == 0, etc...) it's best to keep the implementation simple.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_102993
On Mon May 12 12:07:20 2025 +0000, Rémi Bernon wrote:
Yes, but still I'm not convinced we need to use weak refs. Sure you can add test that show various internal aspects of the implementation but unless there's a good reason (need to break a ref cycle, some application actually depends on returned refcount == 0, etc...) it's best to keep the implementation simple. I that case, should I add todo_wine for all the refcount tests?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_103002
On Mon May 12 12:41:19 2025 +0000, Vibhav Pant wrote:
I that case, should I add todo_wine for all the refcount tests? If you want but in general we also don't care that much about exact ref count values, and such tests can be omitted.
It's sometimes checked more thoroughly for cases where it matters or when you want to understand how components are linked together, or at the end of a test to track leaks. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7997#note_103003
participants (4)
-
Jinoh Kang (＠iamahuman) -
Rémi Bernon
-
Vibhav Pant
-
Vibhav Pant (＠vibhavp)