[PATCH 0/2] MR8555: windows.devices.enumeration: Reimplement DeviceInformationStatics::FindAllAsync and DeviceWatcher using the device query API.
Implement `DeviceWatcher` on top of a `HDEVQUERY` object, obtained from `DevCreateObjectQuery`. All events from the query object's associated callback are directly forwarded to the `DeviceWatcher`'s corresponding event handlers.
As closing/freeing the `HDEVQUERY` is performed asynchronously, I have also added `IWeakReference` support for the device watcher.
From: Vibhav Pant vibhavp@gmail.com
--- dlls/windows.devices.enumeration/Makefile.in | 2 +- dlls/windows.devices.enumeration/main.c | 27 ++++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/dlls/windows.devices.enumeration/Makefile.in b/dlls/windows.devices.enumeration/Makefile.in index 5e3f6fbb6fa..853c1303efa 100644 --- a/dlls/windows.devices.enumeration/Makefile.in +++ b/dlls/windows.devices.enumeration/Makefile.in @@ -1,5 +1,5 @@ MODULE = windows.devices.enumeration.dll -IMPORTS = advapi32 combase setupapi uuid +IMPORTS = advapi32 cfgmgr32 combase setupapi uuid
SOURCES = \ access.c \ diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index c927ccff18d..2cd727a4149 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -21,6 +21,8 @@ #include "initguid.h" #include "private.h" #include "setupapi.h" +#include "devfiltertypes.h" +#include "devquery.h"
#include "wine/debug.h"
@@ -519,12 +521,6 @@ static HRESULT WINAPI device_statics_CreateFromIdAsyncAdditionalProperties( IDev return E_NOTIMPL; }
-static HRESULT append_device_information( IDeviceInformation *info, void *context ) -{ - IVector_IInspectable *vector = context; - return IVector_IInspectable_Append( vector, (IInspectable *)info ); -} - static HRESULT find_all_async( IUnknown *invoker, IUnknown *param, PROPVARIANT *result ) { static const struct vector_iids iids = @@ -536,13 +532,28 @@ static HRESULT find_all_async( IUnknown *invoker, IUnknown *param, PROPVARIANT * }; IVectorView_DeviceInformation *view; IVector_IInspectable *vector; + const DEV_OBJECT *objects; + ULONG len, i; HRESULT hr;
TRACE( "invoker %p, param %p, result %p\n", invoker, param, result );
if (FAILED(hr = vector_create( &iids, (void *)&vector ))) return hr; - hr = enum_device_information( append_device_information, vector ); - + if (FAILED(hr = DevGetObjects( DevObjectTypeDeviceInterfaceDisplay, DevQueryFlagNone, 0, NULL, 0, NULL, &len, &objects ))) + { + IVector_IInspectable_Release( vector ); + return hr; + } + for (i = 0; i < len && SUCCEEDED(hr); i++) + { + IDeviceInformation *info; + if (SUCCEEDED(hr = device_information_create( objects[i].pszObjectId, &info ))) + { + hr = IVector_IInspectable_Append( vector, (IInspectable *)info ); + IDeviceInformation_Release( info ); + } + } + DevFreeObjects( len, objects ); if (SUCCEEDED(hr)) hr = IVector_IInspectable_GetView( vector, (void *)&view ); IVector_IInspectable_Release( vector ); if (FAILED(hr)) return hr;
From: Vibhav Pant vibhavp@gmail.com
--- dlls/windows.devices.enumeration/Makefile.in | 5 +- dlls/windows.devices.enumeration/main.c | 220 +++++++++--------- dlls/windows.devices.enumeration/private.h | 1 + .../tests/devices.c | 4 +- dlls/windows.devices.enumeration/weakref.c | 175 ++++++++++++++ dlls/windows.devices.enumeration/weakref.h | 38 +++ 6 files changed, 334 insertions(+), 109 deletions(-) create mode 100644 dlls/windows.devices.enumeration/weakref.c create mode 100644 dlls/windows.devices.enumeration/weakref.h
diff --git a/dlls/windows.devices.enumeration/Makefile.in b/dlls/windows.devices.enumeration/Makefile.in index 853c1303efa..0a204835ae1 100644 --- a/dlls/windows.devices.enumeration/Makefile.in +++ b/dlls/windows.devices.enumeration/Makefile.in @@ -1,5 +1,5 @@ MODULE = windows.devices.enumeration.dll -IMPORTS = advapi32 cfgmgr32 combase setupapi uuid +IMPORTS = cfgmgr32 combase uuid
SOURCES = \ access.c \ @@ -9,4 +9,5 @@ SOURCES = \ event_handlers.c \ information.c \ main.c \ - vector.c + vector.c \ + weakref.c diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index 2cd727a4149..878c2f71651 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -2,6 +2,7 @@ * * Copyright 2021 Gijs Vermeulen * Copyright 2022 Julian Klemann for CodeWeavers + * Copyright 2025 Vibhav Pant * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -20,7 +21,7 @@
#include "initguid.h" #include "private.h" -#include "setupapi.h" +#include "devpropdef.h" #include "devfiltertypes.h" #include "devquery.h"
@@ -28,62 +29,10 @@
WINE_DEFAULT_DEBUG_CHANNEL(enumeration);
-typedef HRESULT (*enum_device_information_cb)( IDeviceInformation *info, void *context ); - -static HRESULT enum_device_information( enum_device_information_cb callback, void *context ) -{ - HKEY iface_key; - HRESULT hr; - DWORD i; - - if (!(iface_key = SetupDiOpenClassRegKeyExW( NULL, KEY_ENUMERATE_SUB_KEYS, DIOCR_INTERFACE, NULL, NULL ))) - return HRESULT_FROM_WIN32( GetLastError() ); - - for (i = 0, hr = S_OK; SUCCEEDED(hr); i++) - { - char buffer[sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W) + MAX_PATH * sizeof(WCHAR)]; - SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail = (void *)buffer; - SP_DEVICE_INTERFACE_DATA iface = {.cbSize = sizeof(iface)}; - HDEVINFO set = INVALID_HANDLE_VALUE; - GUID iface_class; - WCHAR name[40]; - DWORD j, len; - LSTATUS ret; - - len = ARRAY_SIZE(name); - ret = RegEnumKeyExW( iface_key, i, name, &len, NULL, NULL, NULL, NULL ); - if (ret == ERROR_NO_MORE_ITEMS) break; - if (ret) hr = HRESULT_FROM_WIN32( ret ); - - if (SUCCEEDED(hr) && SUCCEEDED(hr = CLSIDFromString( name, &iface_class ))) - { - set = SetupDiGetClassDevsW( &iface_class, NULL, NULL, DIGCF_DEVICEINTERFACE ); - if (set == INVALID_HANDLE_VALUE) hr = HRESULT_FROM_WIN32( GetLastError() ); - } - - for (j = 0; SUCCEEDED(hr) && SetupDiEnumDeviceInterfaces( set, NULL, &iface_class, j, &iface ); j++) - { - IDeviceInformation *info; - - detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); - if (!SetupDiGetDeviceInterfaceDetailW( set, &iface, detail, sizeof(buffer), NULL, NULL )) continue; - - if (SUCCEEDED(hr = device_information_create( detail->DevicePath, &info ))) - { - hr = callback( info, context ); - IDeviceInformation_Release( info ); - } - } - } - - RegCloseKey( iface_key ); - return hr; -} - struct device_watcher { IDeviceWatcher IDeviceWatcher_iface; - LONG ref; + struct weak_reference_source weak_reference_source;
struct list added_handlers; struct list enumerated_handlers; @@ -92,6 +41,7 @@ struct device_watcher
CRITICAL_SECTION cs; DeviceWatcherStatus status; + HDEVQUERY query; };
static inline struct device_watcher *impl_from_IDeviceWatcher( IDeviceWatcher *iface ) @@ -114,6 +64,13 @@ static HRESULT WINAPI device_watcher_QueryInterface( IDeviceWatcher *iface, REFI return S_OK; }
+ if (IsEqualGUID( iid, &IID_IWeakReferenceSource )) + { + *out = &impl->weak_reference_source.IWeakReferenceSource_iface; + IWeakReferenceSource_AddRef(*out); + return S_OK; + } + FIXME( "%s not implemented, returning E_NOINTERFACE.\n", debugstr_guid( iid ) ); *out = NULL; return E_NOINTERFACE; @@ -122,7 +79,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 = weak_reference_strong_add_ref( &impl->weak_reference_source ); TRACE( "iface %p, ref %lu.\n", iface, ref ); return ref; } @@ -130,7 +87,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 = weak_reference_strong_release( &impl->weak_reference_source ); TRACE( "iface %p, ref %lu.\n", iface, ref );
if (!ref) @@ -141,6 +98,7 @@ static ULONG WINAPI device_watcher_Release( IDeviceWatcher *iface ) WindowsDeleteString( impl->filter ); impl->cs.DebugInfo->Spare[0] = 0; DeleteCriticalSection( &impl->cs ); + if (impl->query) DevCloseObjectQuery( impl->query ); free( impl ); }
@@ -248,37 +206,88 @@ static HRESULT WINAPI device_watcher_get_Status( IDeviceWatcher *iface, DeviceWa return S_OK; }
-static HRESULT add_device_information( IDeviceInformation *info, void *invoker ) +static const char *debugstr_DEV_QUERY_RESULT_ACTION_DATA( const DEV_QUERY_RESULT_ACTION_DATA *data ) { - struct device_watcher *impl = impl_from_IDeviceWatcher( (IDeviceWatcher *)invoker ); - typed_event_handlers_notify( &impl->added_handlers, (IInspectable *)invoker, (IInspectable *)info ); - return S_OK; + const DEV_OBJECT *obj = &data->Data.DeviceObject; + if (!data) return wine_dbg_sprintf( "(null)" ); + if (data->Action == DevQueryResultStateChange) + return wine_dbg_sprintf( "{%d {%d}}", data->Action, data->Data.State ); + return wine_dbg_sprintf( "{%d {{%d %s %lu %p}}}", data->Action, obj->ObjectType, debugstr_w( obj->pszObjectId ), obj->cPropertyCount, obj->pProperties ); }
-static HRESULT device_watcher_start_async( IUnknown *invoker, IUnknown *param, PROPVARIANT *result ) +static void WINAPI device_object_query_callback( HDEVQUERY query, void *data, + const DEV_QUERY_RESULT_ACTION_DATA *action_data ) { - struct device_watcher *impl = impl_from_IDeviceWatcher( (IDeviceWatcher *)invoker ); - DeviceWatcherStatus status; + struct device_watcher *watcher; + IWeakReference *weak = data; + IDeviceWatcher *iface; HRESULT hr;
- hr = enum_device_information( add_device_information, invoker ); + TRACE( "query %p, data %p, action_data %s.\n", query, data, debugstr_DEV_QUERY_RESULT_ACTION_DATA( action_data ) );
- EnterCriticalSection( &impl->cs ); - if (FAILED(hr)) status = DeviceWatcherStatus_Aborted; - else if (impl->status == DeviceWatcherStatus_Stopping) status = DeviceWatcherStatus_Stopped; - else status = DeviceWatcherStatus_EnumerationCompleted; - impl->status = status; - LeaveCriticalSection( &impl->cs ); + if (FAILED(hr = IWeakReference_Resolve( weak, &IID_IDeviceWatcher, (IInspectable **)&iface )) || !iface) + { + if (action_data->Action == DevQueryResultStateChange && + (action_data->Data.State == DevQueryStateClosed || action_data->Data.State == DevQueryStateAborted)) + IWeakReference_Release( weak ); /* No more callbacks are expected, so we can release the weak ref. */ + return; + } + watcher = impl_from_IDeviceWatcher( iface );
- if (status == DeviceWatcherStatus_Stopped) typed_event_handlers_notify( &impl->stopped_handlers, (IInspectable *)invoker, NULL ); - if (status == DeviceWatcherStatus_EnumerationCompleted) typed_event_handlers_notify( &impl->enumerated_handlers, (IInspectable *)invoker, NULL ); - return S_OK; + switch (action_data->Action) + { + case DevQueryResultStateChange: + switch (action_data->Data.State) + { + case DevQueryStateClosed: + case DevQueryStateAborted: + EnterCriticalSection( &watcher->cs ); + if (action_data->Data.State == DevQueryStateAborted) + { + watcher->status = DeviceWatcherStatus_Aborted; + DevCloseObjectQuery( watcher->query ); + } + else + watcher->status = DeviceWatcherStatus_Stopped; + watcher->query = NULL; + LeaveCriticalSection( &watcher->cs ); + + if (action_data->Data.State == DevQueryStateClosed) + typed_event_handlers_notify( &watcher->stopped_handlers, (IInspectable *)iface, NULL ); + IWeakReference_Release( weak ); + break; + case DevQueryStateEnumCompleted: + EnterCriticalSection( &watcher->cs ); + watcher->status = DeviceWatcherStatus_EnumerationCompleted; + LeaveCriticalSection( &watcher->cs ); + + typed_event_handlers_notify( &watcher->enumerated_handlers, (IInspectable *)iface, NULL ); + break; + default: + FIXME( "Unhandled DEV_QUERY_STATE value: %d\n", action_data->Data.State ); + break; + } + break; + case DevQueryResultAdd: + { + IDeviceInformation *info; + if (FAILED(hr = device_information_create( action_data->Data.DeviceObject.pszObjectId, &info ))) + break; + typed_event_handlers_notify( &watcher->added_handlers, (IInspectable *)iface, (IInspectable *)info ); + IDeviceInformation_Release( info ); + break; + } + default: + FIXME( "Unhandled DEV_QUERY_RESULT_ACTION value: %d\n", action_data->Action ); + break; + } + + IDeviceWatcher_Release( iface ); }
static HRESULT WINAPI device_watcher_Start( IDeviceWatcher *iface ) { struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); - IAsyncAction *async; HRESULT hr = S_OK;
FIXME( "iface %p: semi-stub!\n", iface ); @@ -292,17 +301,31 @@ static HRESULT WINAPI device_watcher_Start( IDeviceWatcher *iface ) EnterCriticalSection( &impl->cs ); switch (impl->status) { - case DeviceWatcherStatus_EnumerationCompleted: hr = E_ILLEGAL_METHOD_CALL; break; - case DeviceWatcherStatus_Started: hr = E_ILLEGAL_METHOD_CALL; break; - case DeviceWatcherStatus_Stopping: hr = E_ILLEGAL_METHOD_CALL; break; - default: hr = E_ILLEGAL_METHOD_CALL; - case DeviceWatcherStatus_Aborted: case DeviceWatcherStatus_Created: case DeviceWatcherStatus_Stopped: + { + IWeakReferenceSource *weak_src; + IWeakReference *weak; + HRESULT hr; + + hr = IDeviceWatcher_QueryInterface( iface, &IID_IWeakReferenceSource, (void **)&weak_src ); + if (FAILED(hr)) break; + hr = IWeakReferenceSource_GetWeakReference( weak_src, &weak ); + IWeakReferenceSource_Release( weak_src ); + if (FAILED(hr)) break; + hr = DevCreateObjectQuery( DevObjectTypeDeviceInterfaceDisplay, DevQueryFlagAsyncClose, 0, NULL, 0, NULL, device_object_query_callback, weak, + &impl->query ); + if (FAILED(hr)) + { + IWeakReference_Release( weak ); + break; + } impl->status = DeviceWatcherStatus_Started; - hr = async_action_create( (IUnknown *)iface, device_watcher_start_async, &async ); - if (SUCCEEDED(hr)) IAsyncAction_Release( async ); + break; + } + default: + hr = E_ILLEGAL_METHOD_CALL; break; } LeaveCriticalSection( &impl->cs ); @@ -310,22 +333,9 @@ static HRESULT WINAPI device_watcher_Start( IDeviceWatcher *iface ) return hr; }
-static HRESULT device_watcher_stop_async( IUnknown *invoker, IUnknown *param, PROPVARIANT *result ) -{ - struct device_watcher *impl = impl_from_IDeviceWatcher( (IDeviceWatcher *)invoker ); - - EnterCriticalSection( &impl->cs ); - impl->status = DeviceWatcherStatus_Stopped; - LeaveCriticalSection( &impl->cs ); - - typed_event_handlers_notify( &impl->stopped_handlers, (IInspectable *)invoker, NULL ); - return S_OK; -} - static HRESULT WINAPI device_watcher_Stop( IDeviceWatcher *iface ) { struct device_watcher *impl = impl_from_IDeviceWatcher( iface ); - IAsyncAction *async; HRESULT hr = S_OK;
TRACE( "iface %p\n", iface ); @@ -334,19 +344,14 @@ static HRESULT WINAPI device_watcher_Stop( IDeviceWatcher *iface ) switch (impl->status) { case DeviceWatcherStatus_Aborted: break; - case DeviceWatcherStatus_Created: hr = E_ILLEGAL_METHOD_CALL; break; - case DeviceWatcherStatus_Stopped: hr = E_ILLEGAL_METHOD_CALL; break; - case DeviceWatcherStatus_Stopping: hr = E_ILLEGAL_METHOD_CALL; break; - default: hr = E_ILLEGAL_METHOD_CALL; - case DeviceWatcherStatus_EnumerationCompleted: - impl->status = DeviceWatcherStatus_Stopping; - hr = async_action_create( (IUnknown *)iface, device_watcher_stop_async, &async ); - if (SUCCEEDED(hr)) IAsyncAction_Release( async ); - break; case DeviceWatcherStatus_Started: impl->status = DeviceWatcherStatus_Stopping; - /* an async start is in progress, let it handle stopped state change */ + if (impl->query) + DevCloseObjectQuery( impl->query ); + break; + default: + hr = E_ILLEGAL_METHOD_CALL; break; } LeaveCriticalSection( &impl->cs ); @@ -386,10 +391,15 @@ static HRESULT device_watcher_create( HSTRING filter, IDeviceWatcher **out )
if (!(impl = calloc( 1, sizeof(*impl) ))) return E_OUTOFMEMORY;
- impl->ref = 1; impl->IDeviceWatcher_iface.lpVtbl = &device_watcher_vtbl; + if (FAILED(hr = weak_reference_source_init( &impl->weak_reference_source, (IUnknown *)&impl->IDeviceWatcher_iface ))) + { + free( impl ); + return hr; + } if (FAILED(hr = WindowsDuplicateString( filter, &impl->filter ))) { + weak_reference_strong_release( &impl->weak_reference_source ); free( impl ); return hr; } diff --git a/dlls/windows.devices.enumeration/private.h b/dlls/windows.devices.enumeration/private.h index e0a8e46f8b5..ad15b7916ca 100644 --- a/dlls/windows.devices.enumeration/private.h +++ b/dlls/windows.devices.enumeration/private.h @@ -40,6 +40,7 @@ #include "wine/list.h"
#include "async_private.h" +#include "weakref.h"
struct vector_iids { diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index d9247bb6ced..e669d031d7b 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -401,7 +401,7 @@ static void test_DeviceInformation( void ) ok( status == DeviceWatcherStatus_Started, "got status %u\n", status );
ref = IDeviceWatcher_AddRef( device_watcher ); - todo_wine ok( ref == 2, "got ref %lu\n", ref ); + ok( ref == 2, "got ref %lu\n", ref ); hr = IDeviceWatcher_Stop( device_watcher ); ok( hr == S_OK, "got hr %#lx\n", hr ); ok( !WaitForSingleObject( stopped_handler.event, 1000 ), "wait for stopped_handler.event failed\n" ); @@ -448,7 +448,7 @@ static void test_DeviceInformation( void ) ok( status == DeviceWatcherStatus_Started, "got status %u\n", status );
ref = IDeviceWatcher_AddRef( device_watcher ); - todo_wine ok( ref == 2, "got ref %lu\n", ref ); + ok( ref == 2, "got ref %lu\n", ref ); hr = IDeviceWatcher_Stop( device_watcher ); ok( hr == S_OK, "got hr %#lx\n", hr ); ok( !WaitForSingleObject( stopped_handler.event, 1000 ), "wait for stopped_handler.event failed\n" ); diff --git a/dlls/windows.devices.enumeration/weakref.c b/dlls/windows.devices.enumeration/weakref.c new file mode 100644 index 00000000000..6a10390dd4c --- /dev/null +++ b/dlls/windows.devices.enumeration/weakref.c @@ -0,0 +1,175 @@ +/* WinRT weak reference helpers + * + * Copyright 2025 Zhiyi Zhang for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#define COBJMACROS +#include "weakref.h" + +/* The control block is contained in struct weak_reference so that we don't have to allocate a + * separate struct for it */ +struct weak_reference +{ + IWeakReference IWeakReference_iface; + IUnknown *object; + /* control block */ + LONG ref_strong; + LONG ref_weak; +}; + +static inline struct weak_reference *impl_from_IWeakReference( IWeakReference *iface ) +{ + return CONTAINING_RECORD(iface, struct weak_reference, IWeakReference_iface); +} + +static HRESULT WINAPI weak_reference_QueryInterface( IWeakReference *iface, REFIID iid, void **out ) +{ + struct weak_reference *impl = impl_from_IWeakReference( iface ); + + if (IsEqualGUID( iid, &IID_IUnknown ) || IsEqualGUID( iid, &IID_IWeakReference )) + { + *out = &impl->IWeakReference_iface; + IWeakReference_AddRef( *out ); + return S_OK; + } + + *out = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI weak_reference_AddRef( IWeakReference *iface ) +{ + struct weak_reference *impl = impl_from_IWeakReference( iface ); + return InterlockedIncrement( &impl->ref_weak ); +} + +static ULONG WINAPI weak_reference_Release( IWeakReference *iface ) +{ + struct weak_reference *impl = impl_from_IWeakReference( iface ); + ULONG ref = InterlockedDecrement( &impl->ref_weak ); + + if (!ref) + free( impl ); + return ref; +} + +static HRESULT WINAPI weak_reference_Resolve( IWeakReference *iface, REFIID iid, IInspectable **out ) +{ + struct weak_reference *impl = impl_from_IWeakReference( iface ); + HRESULT hr; + LONG ref; + + *out = NULL; + do + { + if (!(ref = ReadNoFence( &impl->ref_strong ))) + return S_OK; + } while (ref != InterlockedCompareExchange( &impl->ref_strong, ref + 1, ref )); + + hr = IUnknown_QueryInterface( impl->object, iid, (void **)out ); + IUnknown_Release( impl->object ); + return hr; +} + +static const struct IWeakReferenceVtbl weak_reference_vtbl = +{ + weak_reference_QueryInterface, + weak_reference_AddRef, + weak_reference_Release, + weak_reference_Resolve, +}; + +static inline struct weak_reference_source *impl_from_IWeakReferenceSource( IWeakReferenceSource *iface ) +{ + return CONTAINING_RECORD(iface, struct weak_reference_source, IWeakReferenceSource_iface); +} + +static HRESULT WINAPI weak_reference_source_QueryInterface( IWeakReferenceSource *iface, REFIID iid, void **out ) +{ + struct weak_reference_source *weak_reference_source = impl_from_IWeakReferenceSource( iface ); + struct weak_reference *weak_reference = impl_from_IWeakReference( weak_reference_source->weak_reference ); + return IUnknown_QueryInterface( weak_reference->object, iid, out ); +} + +static ULONG WINAPI weak_reference_source_AddRef( IWeakReferenceSource *iface ) +{ + struct weak_reference_source *weak_reference_source = impl_from_IWeakReferenceSource( iface ); + struct weak_reference *weak_reference = impl_from_IWeakReference( weak_reference_source->weak_reference ); + return IUnknown_AddRef( weak_reference->object ); +} + +static ULONG WINAPI weak_reference_source_Release( IWeakReferenceSource *iface ) +{ + struct weak_reference_source *weak_reference_source = impl_from_IWeakReferenceSource( iface ); + struct weak_reference *weak_reference = impl_from_IWeakReference( weak_reference_source->weak_reference ); + return IUnknown_Release( weak_reference->object ); +} + +static HRESULT WINAPI weak_reference_source_GetWeakReference( IWeakReferenceSource *iface, + IWeakReference **weak_reference ) +{ + struct weak_reference_source *impl = impl_from_IWeakReferenceSource( iface ); + + *weak_reference = impl->weak_reference; + IWeakReference_AddRef( *weak_reference ); + return S_OK; +} + +static const struct IWeakReferenceSourceVtbl weak_reference_source_vtbl = +{ + weak_reference_source_QueryInterface, + weak_reference_source_AddRef, + weak_reference_source_Release, + weak_reference_source_GetWeakReference, +}; + +static HRESULT weak_reference_create( IUnknown *object, IWeakReference **out ) +{ + struct weak_reference *weak_reference; + + if (!(weak_reference = calloc( 1, sizeof(*weak_reference) ))) + return E_OUTOFMEMORY; + + weak_reference->IWeakReference_iface.lpVtbl = &weak_reference_vtbl; + weak_reference->ref_strong = 1; + weak_reference->ref_weak = 1; + weak_reference->object = object; + *out = &weak_reference->IWeakReference_iface; + return S_OK; +} + +HRESULT weak_reference_source_init( struct weak_reference_source *source, IUnknown *object ) +{ + source->IWeakReferenceSource_iface.lpVtbl = &weak_reference_source_vtbl; + return weak_reference_create( object, &source->weak_reference ); +} + +ULONG weak_reference_strong_add_ref( struct weak_reference_source *source ) +{ + struct weak_reference *impl = impl_from_IWeakReference( source->weak_reference ); + return InterlockedIncrement( &impl->ref_strong ); +} + +ULONG weak_reference_strong_release( struct weak_reference_source *source ) +{ + struct weak_reference *impl = impl_from_IWeakReference( source->weak_reference ); + ULONG ref = InterlockedDecrement( &impl->ref_strong ); + + if (!ref) + IWeakReference_Release( source->weak_reference ); + return ref; +} diff --git a/dlls/windows.devices.enumeration/weakref.h b/dlls/windows.devices.enumeration/weakref.h new file mode 100644 index 00000000000..7d4bc22b087 --- /dev/null +++ b/dlls/windows.devices.enumeration/weakref.h @@ -0,0 +1,38 @@ +/* WinRT weak reference helpers + * + * Copyright 2025 Zhiyi Zhang for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_WEAKREF_H +#define __WINE_WEAKREF_H + +#include "weakreference.h" + +struct weak_reference_source +{ + IWeakReferenceSource IWeakReferenceSource_iface; + IWeakReference *weak_reference; +}; + +/* Initialize a IWeakReferenceSource with the object to manage */ +HRESULT weak_reference_source_init( struct weak_reference_source *source, IUnknown *object ); +/* Add a strong reference to the managed object */ +ULONG weak_reference_strong_add_ref( struct weak_reference_source *source ); +/* Release a strong reference to the managed object */ +ULONG weak_reference_strong_release( struct weak_reference_source *source ); + +#endif /* __WINE_WEAKREF_H */
As closing/freeing the `HDEVQUERY` is performed asynchronously, I have also added `IWeakReference` support for the device watcher.
If this is only meant to fix some tests I don't think it's worth the added complexity. We only care about matching Windows (weak) refcounting when necessary: if an application depends on it, or if it is required to break reference cycle.
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/main.c:
switch (impl->status) { case DeviceWatcherStatus_Aborted: break;
- case DeviceWatcherStatus_Created: hr = E_ILLEGAL_METHOD_CALL; break;
- case DeviceWatcherStatus_Stopped: hr = E_ILLEGAL_METHOD_CALL; break;
- case DeviceWatcherStatus_Stopping: hr = E_ILLEGAL_METHOD_CALL; break;
- default: hr = E_ILLEGAL_METHOD_CALL;
I think it's cleaner to keep the explicit list of handled states rather than hiding them in the default case. The default case itself should maybe even do `assert( 0 )` instead, but it's probably not useful to change it at this point.
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/main.c:
case DevQueryStateAborted:EnterCriticalSection( &watcher->cs );if (action_data->Data.State == DevQueryStateAborted){watcher->status = DeviceWatcherStatus_Aborted;DevCloseObjectQuery( watcher->query );}elsewatcher->status = DeviceWatcherStatus_Stopped;watcher->query = NULL;LeaveCriticalSection( &watcher->cs );if (action_data->Data.State == DevQueryStateClosed)typed_event_handlers_notify( &watcher->stopped_handlers, (IInspectable *)iface, NULL );IWeakReference_Release( weak );break;
This doesn't look very useful to merge both cases after all, what about:
```suggestion:-16+0 case DevQueryStateClosed: EnterCriticalSection( &watcher->cs ); watcher->status = DeviceWatcherStatus_Stopped; watcher->query = NULL; LeaveCriticalSection( &watcher->cs ); typed_event_handlers_notify( &watcher->stopped_handlers, (IInspectable *)iface, NULL ); IWeakReference_Release( weak ); break; case DevQueryStateAborted: EnterCriticalSection( &watcher->cs ); watcher->status = DeviceWatcherStatus_Aborted; DevCloseObjectQuery( watcher->query ); watcher->query = NULL; LeaveCriticalSection( &watcher->cs ); IWeakReference_Release( weak ); break; ```
On Wed Jul 30 08:13:21 2025 +0000, Rémi Bernon wrote:
I think it's cleaner to keep the explicit list of handled states rather than hiding them in the default case. The default case itself should maybe even do `assert( 0 )` instead, but it's probably not useful to change it at this point.
Ah, that makes sense. I have reverted this, thanks.
On Wed Jul 30 08:13:21 2025 +0000, Rémi Bernon wrote:
This doesn't look very useful to merge both cases after all, what about:
case DevQueryStateClosed: EnterCriticalSection( &watcher->cs ); watcher->status = DeviceWatcherStatus_Stopped; watcher->query = NULL; LeaveCriticalSection( &watcher->cs ); typed_event_handlers_notify( &watcher->stopped_handlers, (IInspectable *)iface, NULL ); IWeakReference_Release( weak ); break; case DevQueryStateAborted: EnterCriticalSection( &watcher->cs ); watcher->status = DeviceWatcherStatus_Aborted; DevCloseObjectQuery( watcher->query ); watcher->query = NULL; LeaveCriticalSection( &watcher->cs ); IWeakReference_Release( weak ); break;
Yeah, there isn't much common code between them. Thanks.
-
Rémi Bernon (＠rbernon) -
Vibhav Pant
-
Vibhav Pant (＠vibhavp)