[PATCH 0/5] MR8685: wintypes: Implement IMap::{Lookup, HasKey, Insert} for IPropertySet.
`IMap` is implemented on top of a `rb_tree` guarded with an `SRWLOCK`. We need the map to be thread safe as: * `PropertySet` is marked as `IAgileObject`. * I plan to use `PropertySet` as the backing store for [`DeviceInformation::Properties`](https://learn.microsoft.com/en-us/uwp/api/windows.devices.enumeration.device...). These properties can be updated by apps after receiving an [`Updated`](https://learn.microsoft.com/en-us/uwp/api/windows.devices.enumeration.device...) event from the device watcher with [`DeviceInformation::Update`](https://learn.microsoft.com/en-us/uwp/api/windows.devices.enumeration.device...). The current `DeviceWatcher` implementation dispatches the event delegates for `Update` from a separate thread. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/wintypes/propertyset.c | 3 ++- dlls/wintypes/tests/wintypes.c | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/wintypes/propertyset.c b/dlls/wintypes/propertyset.c index 6a0ccc30c2b..ad4d54a9cfb 100644 --- a/dlls/wintypes/propertyset.c +++ b/dlls/wintypes/propertyset.c @@ -49,7 +49,8 @@ static HRESULT STDMETHODCALLTYPE propertyset_QueryInterface( IPropertySet *iface *out = NULL; if (IsEqualGUID( iid, &IID_IUnknown ) || IsEqualGUID( iid, &IID_IInspectable ) || - IsEqualGUID( iid, &IID_IPropertySet )) + IsEqualGUID( iid, &IID_IPropertySet ) || + IsEqualGUID( iid, &IID_IAgileObject )) { *out = iface; IUnknown_AddRef( (IUnknown *)*out ); diff --git a/dlls/wintypes/tests/wintypes.c b/dlls/wintypes/tests/wintypes.c index ebdf5acbadf..10646a48829 100644 --- a/dlls/wintypes/tests/wintypes.c +++ b/dlls/wintypes/tests/wintypes.c @@ -1288,6 +1288,8 @@ static void test_IPropertySet(void) IInspectable_Release( inspectable ); ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + check_interface( propset, &IID_IAgileObject, TRUE ); + hr = IPropertySet_QueryInterface( propset, &IID_IObservableMap_HSTRING_IInspectable, (void **)&observable_map ); ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); IObservableMap_HSTRING_IInspectable_Release( observable_map ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8685
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/wintypes/tests/wintypes.c | 148 ++++++++++++++++++++++++++++++++- 1 file changed, 146 insertions(+), 2 deletions(-) diff --git a/dlls/wintypes/tests/wintypes.c b/dlls/wintypes/tests/wintypes.c index 10646a48829..0d8d80ac372 100644 --- a/dlls/wintypes/tests/wintypes.c +++ b/dlls/wintypes/tests/wintypes.c @@ -1256,15 +1256,17 @@ static void test_IPropertySet(void) { static const WCHAR *class_name = RuntimeClass_Windows_Foundation_Collections_PropertySet; IActivationFactory *propset_factory; - IInspectable *inspectable; + IPropertyValueStatics *propval_statics; + IInspectable *inspectable, *val, *val1, *val2, *val3; IPropertySet *propset; IMap_HSTRING_IInspectable *map; IMapView_HSTRING_IInspectable *map_view; IObservableMap_HSTRING_IInspectable *observable_map; IIterable_IKeyValuePair_HSTRING_IInspectable *iterable; IIterator_IKeyValuePair_HSTRING_IInspectable *iterator; + BOOLEAN boolean; HRESULT hr; - HSTRING name; + HSTRING name, key1, key2; hr = RoInitialize( RO_INIT_MULTITHREADED ); ok( hr == S_OK, "got %#lx\n", hr ); @@ -1280,6 +1282,19 @@ static void test_IPropertySet(void) goto done; } + class_name = RuntimeClass_Windows_Foundation_PropertyValue; + hr = WindowsCreateString( class_name, wcslen( class_name ), &name ); + ok( hr == S_OK, "got %#lx\n", hr ); + hr = RoGetActivationFactory( name, &IID_IPropertyValueStatics, (void **)&propval_statics); + WindowsDeleteString( name ); + ok( hr == S_OK || broken( hr == REGDB_E_CLASSNOTREG ), "RoGetActivationFactory failed, hr %#lx.\n", hr ); + if (hr != S_OK) + { + win_skip( "%s runtimeclass not registered, skipping tests.\n", wine_dbgstr_w( class_name ) ); + IActivationFactory_Release( propset_factory ); + goto done; + } + hr = IActivationFactory_ActivateInstance( propset_factory, &inspectable ); IActivationFactory_Release( propset_factory ); ok( hr == S_OK, "got %#lx\n", hr ); @@ -1298,6 +1313,134 @@ static void test_IPropertySet(void) IPropertySet_Release( propset ); ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + boolean = TRUE; + hr = IMap_HSTRING_IInspectable_HasKey( map, NULL, &boolean ); + todo_wine + ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); + todo_wine + ok( !boolean, "Got boolean %d.\n", boolean ); + hr = IMap_HSTRING_IInspectable_Lookup( map, NULL, &val ); + todo_wine + ok( hr == E_BOUNDS, "Got hr %#lx\n", hr ); + + hr = IPropertyValueStatics_CreateUInt32( propval_statics, 0xdeadbeef, &val1 ); + ok( hr == S_OK, "CreateUInt32 failed, got %#lx\n", hr ); + boolean = TRUE; + hr = IMap_HSTRING_IInspectable_Insert( map, NULL, val1, &boolean ); + todo_wine + ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); + todo_wine + ok( !boolean, "Got boolean %d.\n", boolean ); + + hr = IPropertyValueStatics_CreateUInt32( propval_statics, 0xc0decafe, &val2 ); + ok( hr == S_OK, "CreateUInt32 failed, got %#lx\n", hr ); + boolean = FALSE; + hr = IMap_HSTRING_IInspectable_Insert( map, NULL, val2, &boolean ); + IInspectable_Release( val2 ); + todo_wine + ok( boolean, "Got boolean %d.\n", boolean ); + todo_wine + ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); + boolean = FALSE; + hr = IMap_HSTRING_IInspectable_HasKey( map, NULL, &boolean ); + todo_wine + ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); + todo_wine + ok( boolean, "Got boolean %d.\n", boolean ); + hr = IMap_HSTRING_IInspectable_Lookup( map, NULL, &val ); + todo_wine + ok( hr == S_OK, "Lookup failed, got %#lx\n", hr ); + if (SUCCEEDED(hr)) + { + IPropertyValue *propval; + UINT32 uint32 = 0; + + hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); + IInspectable_Release( val ); + ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + hr = IPropertyValue_GetUInt32( propval, &uint32 ); + IPropertyValue_Release( propval ); + ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); + ok( uint32 == 0xc0decafe, "Got uint32 %u\n", uint32 ); + } + + hr = WindowsCreateString( L"foo", 3, &key1 ); + ok( hr == S_OK, "WindowsCreateString failed, got %#lx\n", hr ); + boolean = TRUE; + hr = IMap_HSTRING_IInspectable_Lookup( map, key1, &val ); + todo_wine + ok( hr == E_BOUNDS, "Got hr %#lx\n", hr ); + hr = IMap_HSTRING_IInspectable_Insert( map, key1, val1, &boolean ); + IInspectable_Release( val1 ); + todo_wine + ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); + todo_wine + ok( !boolean, "Got boolean %d.\n", boolean ); + boolean = FALSE; + hr = IMap_HSTRING_IInspectable_HasKey( map, key1, &boolean ); + todo_wine + ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); + todo_wine + ok( boolean, "Got boolean %d.\n", boolean ); + hr = IMap_HSTRING_IInspectable_Lookup( map, key1, &val ); + todo_wine + ok( hr == S_OK, "Lookup failed, got %#lx\n", hr ); + if (SUCCEEDED(hr)) + { + IPropertyValue *propval; + UINT32 uint32 = 0; + + hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); + IInspectable_Release( val ); + ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + hr = IPropertyValue_GetUInt32( propval, &uint32 ); + IPropertyValue_Release( propval ); + ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); + ok( uint32 == 0xdeadbeef, "Got uint32 %u\n", uint32 ); + } + WindowsDeleteString( key1 ); + + hr = WindowsCreateString( L"bar", 3, &key2 ); + ok( hr == S_OK, "WindowsCreateString failed, got %#lx\n", hr ); + boolean = TRUE; + hr = IMap_HSTRING_IInspectable_HasKey( map, key2, &boolean ); + todo_wine + ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); + todo_wine + ok( !boolean, "Got boolean %d.\n", boolean ); + hr = IPropertyValueStatics_CreateUInt64( propval_statics, 0xdeadbeefdeadbeef, &val3 ); + ok( hr == S_OK, "CreateUInt32 failed, got %#lx\n", hr ); + boolean = TRUE; + hr = IMap_HSTRING_IInspectable_Insert( map, key2, val3, &boolean ); + IInspectable_Release( val3 ); + todo_wine + ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); + todo_wine + ok( !boolean, "Got boolean %d.\n", boolean ); + boolean = FALSE; + hr = IMap_HSTRING_IInspectable_HasKey( map, key2, &boolean ); + todo_wine + ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); + todo_wine + ok( boolean, "Got boolean %d.\n", boolean ); + hr = IMap_HSTRING_IInspectable_Lookup( map, key2, &val ); + todo_wine + ok( hr == S_OK, "Lookup failed, got %#lx\n", hr ); + if (SUCCEEDED(hr)) + { + IPropertyValue *propval; + UINT64 uint64 = 0; + + hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); + IInspectable_Release( val ); + ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + hr = IPropertyValue_GetUInt64( propval, &uint64 ); + IPropertyValue_Release( propval ); + ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); + ok( uint64 == 0xdeadbeefdeadbeef, "Got uint64 %I64u\n", uint64 ); + } + WindowsDeleteString( key2 ); + hr = IMap_HSTRING_IInspectable_QueryInterface( map, &IID_IIterable_IKeyValuePair_HSTRING_IInspectable, (void **)&iterable ); ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); @@ -1329,6 +1472,7 @@ static void test_IPropertySet(void) IMapView_HSTRING_IInspectable_Release( map_view ); } IMap_HSTRING_IInspectable_Release( map ); + IPropertyValueStatics_Release( propval_statics ); done: RoUninitialize(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8685
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/wintypes/propertyset.c | 69 +++++++++++++++++++++++++++++++++- dlls/wintypes/tests/wintypes.c | 8 ---- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/dlls/wintypes/propertyset.c b/dlls/wintypes/propertyset.c index ad4d54a9cfb..0b2afdff8af 100644 --- a/dlls/wintypes/propertyset.c +++ b/dlls/wintypes/propertyset.c @@ -24,14 +24,42 @@ #include "private.h" +#include <wine/rbtree.h> + WINE_DEFAULT_DEBUG_CHANNEL( wintypes ); +struct map_entry +{ + struct rb_entry entry; + HSTRING key; + IInspectable *value; +}; + +static int map_entry_compare( const void *key, const struct rb_entry *entry ) +{ + INT32 order; + + WindowsCompareStringOrdinal( (HSTRING)key, (HSTRING)RB_ENTRY_VALUE( entry, struct map_entry, entry )->key, &order ); + return order; +} + +static void map_entry_destroy( struct rb_entry *entry, void *data ) +{ + struct map_entry *map_entry = RB_ENTRY_VALUE( entry, struct map_entry, entry ); + + WindowsDeleteString( map_entry->key ); + IInspectable_Release( map_entry->value ); + free( entry ); +} + struct propertyset { IPropertySet IPropertySet_iface; IObservableMap_HSTRING_IInspectable IObservableMap_HSTRING_IInspectable_iface; IMap_HSTRING_IInspectable IMap_HSTRING_IInspectable_iface; IIterable_IKeyValuePair_HSTRING_IInspectable IIterable_IKeyValuePair_HSTRING_IInspectable_iface; + SRWLOCK lock; + struct rb_tree entries; /* Guarded by lock */ LONG ref; }; @@ -97,7 +125,10 @@ static ULONG STDMETHODCALLTYPE propertyset_Release( IPropertySet *iface ) ref = InterlockedDecrement( &impl->ref ); if (!ref) + { + rb_destroy( &impl->entries, map_entry_destroy, NULL ); free( impl ); + } return ref; } @@ -191,8 +222,40 @@ static HRESULT STDMETHODCALLTYPE propertyset_GetView( IMap_HSTRING_IInspectable static HRESULT STDMETHODCALLTYPE propertyset_Insert( IMap_HSTRING_IInspectable *iface, HSTRING key, IInspectable *value, boolean *replaced ) { - FIXME( "(%p, %s, %p, %p) stub!\n", iface, debugstr_hstring( key ), value, replaced ); - return E_NOTIMPL; + struct propertyset *impl = impl_from_IMap_HSTRING_IInspectable( iface ); + struct map_entry *map_entry; + struct rb_entry *entry; + HRESULT hr = S_OK; + + TRACE( "(%p, %s, %p, %p)\n", iface, debugstr_hstring( key ), value, replaced ); + + *replaced = FALSE; + AcquireSRWLockExclusive( &impl->lock ); + if (!(entry = rb_get( &impl->entries, key ))) + { + if (!(map_entry = calloc( 1, sizeof( *map_entry ) ))) + { + hr = E_OUTOFMEMORY; + goto done; + } + if (FAILED(hr = WindowsDuplicateString( key, &map_entry->key ))) + { + free( map_entry ); + goto done; + } + rb_put( &impl->entries, key, &map_entry->entry ); + } + else + { + map_entry = RB_ENTRY_VALUE( entry, struct map_entry, entry ); + IInspectable_Release( map_entry->value ); + *replaced = TRUE; + } + map_entry->value = value; + IInspectable_AddRef( value ); +done: + ReleaseSRWLockExclusive( &impl->lock ); + return hr; } static HRESULT STDMETHODCALLTYPE propertyset_Remove( IMap_HSTRING_IInspectable *iface, HSTRING key ) @@ -327,6 +390,8 @@ static HRESULT STDMETHODCALLTYPE factory_ActivateInstance( IActivationFactory *i impl->IObservableMap_HSTRING_IInspectable_iface.lpVtbl = &propertyset_IObservableMap_vtbl; impl->IMap_HSTRING_IInspectable_iface.lpVtbl = &propertyset_IMap_vtbl; impl->IIterable_IKeyValuePair_HSTRING_IInspectable_iface.lpVtbl = &iterable_kvpair_HSTRING_IInspectable_vtbl; + InitializeSRWLock( &impl->lock ); + rb_init( &impl->entries, map_entry_compare ); impl->ref = 1; *instance = (IInspectable *)&impl->IPropertySet_iface; return S_OK; diff --git a/dlls/wintypes/tests/wintypes.c b/dlls/wintypes/tests/wintypes.c index 0d8d80ac372..9e1d81404fe 100644 --- a/dlls/wintypes/tests/wintypes.c +++ b/dlls/wintypes/tests/wintypes.c @@ -1327,9 +1327,7 @@ static void test_IPropertySet(void) ok( hr == S_OK, "CreateUInt32 failed, got %#lx\n", hr ); boolean = TRUE; hr = IMap_HSTRING_IInspectable_Insert( map, NULL, val1, &boolean ); - todo_wine ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); - todo_wine ok( !boolean, "Got boolean %d.\n", boolean ); hr = IPropertyValueStatics_CreateUInt32( propval_statics, 0xc0decafe, &val2 ); @@ -1337,9 +1335,7 @@ static void test_IPropertySet(void) boolean = FALSE; hr = IMap_HSTRING_IInspectable_Insert( map, NULL, val2, &boolean ); IInspectable_Release( val2 ); - todo_wine ok( boolean, "Got boolean %d.\n", boolean ); - todo_wine ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); boolean = FALSE; hr = IMap_HSTRING_IInspectable_HasKey( map, NULL, &boolean ); @@ -1372,9 +1368,7 @@ static void test_IPropertySet(void) ok( hr == E_BOUNDS, "Got hr %#lx\n", hr ); hr = IMap_HSTRING_IInspectable_Insert( map, key1, val1, &boolean ); IInspectable_Release( val1 ); - todo_wine ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); - todo_wine ok( !boolean, "Got boolean %d.\n", boolean ); boolean = FALSE; hr = IMap_HSTRING_IInspectable_HasKey( map, key1, &boolean ); @@ -1413,9 +1407,7 @@ static void test_IPropertySet(void) boolean = TRUE; hr = IMap_HSTRING_IInspectable_Insert( map, key2, val3, &boolean ); IInspectable_Release( val3 ); - todo_wine ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); - todo_wine ok( !boolean, "Got boolean %d.\n", boolean ); boolean = FALSE; hr = IMap_HSTRING_IInspectable_HasKey( map, key2, &boolean ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8685
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/wintypes/propertyset.c | 10 ++++++++-- dlls/wintypes/tests/wintypes.c | 10 ---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/dlls/wintypes/propertyset.c b/dlls/wintypes/propertyset.c index 0b2afdff8af..579aa5cdc60 100644 --- a/dlls/wintypes/propertyset.c +++ b/dlls/wintypes/propertyset.c @@ -210,8 +210,14 @@ static HRESULT STDMETHODCALLTYPE propertyset_get_Size( IMap_HSTRING_IInspectable static HRESULT STDMETHODCALLTYPE propertyset_HasKey( IMap_HSTRING_IInspectable *iface, HSTRING key, boolean *exists ) { - FIXME( "(%p, %s, %p) stub!\n", iface, debugstr_hstring( key ), exists ); - return E_NOTIMPL; + struct propertyset *impl = impl_from_IMap_HSTRING_IInspectable( iface ); + + TRACE( "(%p, %s, %p)\n", iface, debugstr_hstring( key ), exists ); + + AcquireSRWLockExclusive( &impl->lock); + *exists = !!rb_get( &impl->entries, key ); + ReleaseSRWLockExclusive( &impl->lock ); + return S_OK; } static HRESULT STDMETHODCALLTYPE propertyset_GetView( IMap_HSTRING_IInspectable *iface, IMapView_HSTRING_IInspectable **view ) diff --git a/dlls/wintypes/tests/wintypes.c b/dlls/wintypes/tests/wintypes.c index 9e1d81404fe..5dc95648bb0 100644 --- a/dlls/wintypes/tests/wintypes.c +++ b/dlls/wintypes/tests/wintypes.c @@ -1315,9 +1315,7 @@ static void test_IPropertySet(void) boolean = TRUE; hr = IMap_HSTRING_IInspectable_HasKey( map, NULL, &boolean ); - todo_wine ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); - todo_wine ok( !boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, NULL, &val ); todo_wine @@ -1339,9 +1337,7 @@ static void test_IPropertySet(void) ok( hr == S_OK, "Insert failed, got %#lx\n", hr ); boolean = FALSE; hr = IMap_HSTRING_IInspectable_HasKey( map, NULL, &boolean ); - todo_wine ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); - todo_wine ok( boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, NULL, &val ); todo_wine @@ -1372,9 +1368,7 @@ static void test_IPropertySet(void) ok( !boolean, "Got boolean %d.\n", boolean ); boolean = FALSE; hr = IMap_HSTRING_IInspectable_HasKey( map, key1, &boolean ); - todo_wine ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); - todo_wine ok( boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, key1, &val ); todo_wine @@ -1398,9 +1392,7 @@ static void test_IPropertySet(void) ok( hr == S_OK, "WindowsCreateString failed, got %#lx\n", hr ); boolean = TRUE; hr = IMap_HSTRING_IInspectable_HasKey( map, key2, &boolean ); - todo_wine ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); - todo_wine ok( !boolean, "Got boolean %d.\n", boolean ); hr = IPropertyValueStatics_CreateUInt64( propval_statics, 0xdeadbeefdeadbeef, &val3 ); ok( hr == S_OK, "CreateUInt32 failed, got %#lx\n", hr ); @@ -1411,9 +1403,7 @@ static void test_IPropertySet(void) ok( !boolean, "Got boolean %d.\n", boolean ); boolean = FALSE; hr = IMap_HSTRING_IInspectable_HasKey( map, key2, &boolean ); - todo_wine ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); - todo_wine ok( boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, key2, &val ); todo_wine -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8685
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/wintypes/propertyset.c | 24 +++++++++++- dlls/wintypes/tests/wintypes.c | 71 +++++++++++++--------------------- 2 files changed, 49 insertions(+), 46 deletions(-) diff --git a/dlls/wintypes/propertyset.c b/dlls/wintypes/propertyset.c index 579aa5cdc60..d92893af5c8 100644 --- a/dlls/wintypes/propertyset.c +++ b/dlls/wintypes/propertyset.c @@ -198,8 +198,28 @@ DEFINE_IINSPECTABLE( propertyset_IMap, IMap_HSTRING_IInspectable, struct propert static HRESULT STDMETHODCALLTYPE propertyset_Lookup( IMap_HSTRING_IInspectable *iface, HSTRING key, IInspectable **value ) { - FIXME( "(%p, %s, %p) stub!\n", iface, debugstr_hstring( key ), value ); - return E_NOTIMPL; + struct propertyset *impl = impl_from_IMap_HSTRING_IInspectable( iface ); + struct rb_entry *entry; + HRESULT hr; + + TRACE( "(%p, %s, %p)\n", iface, debugstr_hstring( key ), value ); + + AcquireSRWLockExclusive( &impl->lock ); + entry = rb_get( &impl->entries, key ); + if (!entry) + { + *value = NULL; + hr = E_BOUNDS; + } + else + { + *value = RB_ENTRY_VALUE( entry, struct map_entry, entry )->value; + IInspectable_AddRef( *value ); + hr = S_OK; + } + ReleaseSRWLockExclusive( &impl->lock ); + + return hr; } static HRESULT STDMETHODCALLTYPE propertyset_get_Size( IMap_HSTRING_IInspectable *iface, UINT32 *size ) diff --git a/dlls/wintypes/tests/wintypes.c b/dlls/wintypes/tests/wintypes.c index 5dc95648bb0..47d09aac792 100644 --- a/dlls/wintypes/tests/wintypes.c +++ b/dlls/wintypes/tests/wintypes.c @@ -1264,6 +1264,9 @@ static void test_IPropertySet(void) IObservableMap_HSTRING_IInspectable *observable_map; IIterable_IKeyValuePair_HSTRING_IInspectable *iterable; IIterator_IKeyValuePair_HSTRING_IInspectable *iterator; + IPropertyValue *propval; + UINT32 uint32; + UINT64 uint64; BOOLEAN boolean; HRESULT hr; HSTRING name, key1, key2; @@ -1318,7 +1321,6 @@ static void test_IPropertySet(void) ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); ok( !boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, NULL, &val ); - todo_wine ok( hr == E_BOUNDS, "Got hr %#lx\n", hr ); hr = IPropertyValueStatics_CreateUInt32( propval_statics, 0xdeadbeef, &val1 ); @@ -1340,27 +1342,20 @@ static void test_IPropertySet(void) ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); ok( boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, NULL, &val ); - todo_wine ok( hr == S_OK, "Lookup failed, got %#lx\n", hr ); - if (SUCCEEDED(hr)) - { - IPropertyValue *propval; - UINT32 uint32 = 0; - - hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); - IInspectable_Release( val ); - ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); - hr = IPropertyValue_GetUInt32( propval, &uint32 ); - IPropertyValue_Release( propval ); - ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); - ok( uint32 == 0xc0decafe, "Got uint32 %u\n", uint32 ); - } + hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); + IInspectable_Release( val ); + ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + uint32 = 0; + hr = IPropertyValue_GetUInt32( propval, &uint32 ); + IPropertyValue_Release( propval ); + ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); + ok( uint32 == 0xc0decafe, "Got uint32 %u\n", uint32 ); hr = WindowsCreateString( L"foo", 3, &key1 ); ok( hr == S_OK, "WindowsCreateString failed, got %#lx\n", hr ); boolean = TRUE; hr = IMap_HSTRING_IInspectable_Lookup( map, key1, &val ); - todo_wine ok( hr == E_BOUNDS, "Got hr %#lx\n", hr ); hr = IMap_HSTRING_IInspectable_Insert( map, key1, val1, &boolean ); IInspectable_Release( val1 ); @@ -1371,21 +1366,15 @@ static void test_IPropertySet(void) ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); ok( boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, key1, &val ); - todo_wine ok( hr == S_OK, "Lookup failed, got %#lx\n", hr ); - if (SUCCEEDED(hr)) - { - IPropertyValue *propval; - UINT32 uint32 = 0; - - hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); - IInspectable_Release( val ); - ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); - hr = IPropertyValue_GetUInt32( propval, &uint32 ); - IPropertyValue_Release( propval ); - ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); - ok( uint32 == 0xdeadbeef, "Got uint32 %u\n", uint32 ); - } + hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); + IInspectable_Release( val ); + ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + uint32 = 0; + hr = IPropertyValue_GetUInt32( propval, &uint32 ); + IPropertyValue_Release( propval ); + ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); + ok( uint32 == 0xdeadbeef, "Got uint32 %u\n", uint32 ); WindowsDeleteString( key1 ); hr = WindowsCreateString( L"bar", 3, &key2 ); @@ -1406,21 +1395,15 @@ static void test_IPropertySet(void) ok( hr == S_OK, "HasKey failed, got %#lx\n", hr ); ok( boolean, "Got boolean %d.\n", boolean ); hr = IMap_HSTRING_IInspectable_Lookup( map, key2, &val ); - todo_wine ok( hr == S_OK, "Lookup failed, got %#lx\n", hr ); - if (SUCCEEDED(hr)) - { - IPropertyValue *propval; - UINT64 uint64 = 0; - - hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); - IInspectable_Release( val ); - ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); - hr = IPropertyValue_GetUInt64( propval, &uint64 ); - IPropertyValue_Release( propval ); - ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); - ok( uint64 == 0xdeadbeefdeadbeef, "Got uint64 %I64u\n", uint64 ); - } + hr = IInspectable_QueryInterface( val, &IID_IPropertyValue, (void **)&propval ); + IInspectable_Release( val ); + ok( hr == S_OK, "QueryInterface failed, got %#lx\n", hr ); + uint64 = 0; + hr = IPropertyValue_GetUInt64( propval, &uint64 ); + IPropertyValue_Release( propval ); + ok( hr == S_OK, "GetUInt32, got %#lx\n", hr ); + ok( uint64 == 0xdeadbeefdeadbeef, "Got uint64 %I64u\n", uint64 ); WindowsDeleteString( key2 ); hr = IMap_HSTRING_IInspectable_QueryInterface( map, &IID_IIterable_IKeyValuePair_HSTRING_IInspectable, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8685
Is it an api requirement to return IMap objects? From your links I can see only some dictionary interfaces, is that the same thing? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_111795
On Fri Aug 1 10:33:35 2025 +0000, Nikolay Sivov wrote:
Is it an api requirement to return IMap objects? From your links I can see only some dictionary interfaces, is that the same thing? You're right, `Properties` is an IMapView, not `IMap`. However, since the underlying properties can be updated, we would still need an `IMap`-like collection of (HSTRING, IInspectable) pairs to store properties and implement `Update`. Instead of duplicating an `IMapView` implementation for Windows.Devices.Enumeration, I feel this is a better approach IMO.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_111796
On Fri Aug 1 10:36:23 2025 +0000, Vibhav Pant wrote:
You're right, `Properties` is an `IMapView,` not `IMap`. However, since the underlying properties can be updated, we would still need an `IMap`-like collection of \`(HSTRING, IInspectable)\` pairs to store properties and implement `Update`. Instead of duplicating an `IMapView` implementation for Windows.Devices.Enumeration, I feel this is a better approach IMO. Sure, it's better to reuse when we can. What I don't understand is how this reuse happens normally, is it usually source-based on winrt SDK, so you compile in some template everywhere, or libraries are expected to go through CoCreate* equivalent to import something like IMap which is so basic that it belongs to a standard library, if that exists for winrt.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_111797
or libraries are expected to go through CoCreate\* equivalent to import something like IMap which is so basic that it belongs to a standard library, if that exists for winrt.
Yeah, at least that is what I wish to do. The idea is that dlls/windows.devices.enumeration/information.c:device_information_create would roughly look something like this: ```c HRESULT device_information_create( const WCHAR *path, const DEVPROPERTY **properties, ULONG len, IDeviceInformation **info ) { struct device_information *impl; IPropertyValueStatics *statics; ULONG i; /* initialize impl fields */ RoActivateInstance( "Windows.Foundation.Collections.PropertySet", &impl->properties); RoGetActivationFactory( "Windows.Foundation.Collections.PropertyValue", &IID_PropertyValueStatics, &statics); for (i = 0; i < len; i++) { WCHAR *name; HSTRING key; IInspectable *val; PSGetNameFromPropertyKey( &properties[i].CompKey.Key, &name); WindowsCreateString( name, wcslen(name), &key ); switch (properties[i].Type) { case DEVPROP_TYPE_INT16: IPropertyValueStatics_CreateInt16( statics, (INT16 *)properties[i].Buffer, &val ); break; /* Handle other DEVPROP_TYPE_* -> IPropertyValue conversions */ } IPropertySet_Insert( impl->properties, key, val ); IInspectable_Release( val ); } IPropertyValueStatics_Release( statics ); *info = &impl->IDeviceInformation_iface; return S_OK; } ``` So, even if Windows.Devices.Enumeration has its own key-value collection code, we would still need to "import" (I think the term is "activate" in WinRT parlance) [`Windows.Foundation.Collections.PropertyValue`](https://learn.microsoft.com/en-us/uwp/api/windows.foundation.ipropertyvalue?...) for creating boxed WinRT values. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_111801
I am not that familiar with this area to give a review. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_112439
I'm not sure this is supposed to be thread safe, I've been able to make the test crash by hammering the map with Insert / Remove calls from multiple threads. Similar to the IVector interface, it looks like this is usually implemented directly in the SDK, using C++ STL wrappers. I think we should implement it with some shareable code as well, like we did for IVector (although it's going to be difficult to be truly compatible with STL containers if we ever need to be). Then it's not obvious to me that our `rb_tree` is the best tool to implement this either, some methods seem to be inconvenient. It also looks like the SDK provides both ordered or unordered IMap variants. What about something like https://gitlab.winehq.org/wine/wine/-/merge_requests/8749 instead? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_112619
This merge request was closed by Vibhav Pant. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685
On Tue Aug 26 14:45:23 2025 +0000, Rémi Bernon wrote:
I'm not sure this is supposed to be thread safe, I've been able to make the test crash by hammering the map with Insert / Remove calls from multiple threads. Similar to the IVector interface, it looks like this is usually implemented directly in the SDK, using C++ STL wrappers. I think we should implement it with some shareable code as well, like we did for IVector (although it's going to be difficult to be truly compatible with STL containers if we ever need to be). Then it's not obvious to me that our `rb_tree` is the best tool to implement this either, some methods seem to be inconvenient. It also looks like the SDK provides both ordered or unordered IMap variants. What about something like https://gitlab.winehq.org/wine/wine/-/merge_requests/8749 instead? Ah, `rb_tree` could make support iterators trickier, this looks much better. Thanks :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_113897
On Tue Aug 26 14:45:23 2025 +0000, Vibhav Pant wrote:
Ah, `rb_tree` could make support iterators trickier, this looks much better. Thanks :) Well fwiw I'm not sure anymore about the thread safety, like you said it seems it is supposed to be guarded if we consider the Rust projection. OTOH the SDK headers also are apparently quite obviously unsafe at least in the IMap templated implementation. I checked again my tests and it's very possible that the crash I was able to trigger was caused by some overlook on my part (I had a thread wait timeout and perhaps it went past it, releasing the map while threads were still using it).
Overall it looks like either PropertySet has a dedicated, safe, IMap implementation, or it adds a lock around every generic unsafe IMap calls (which maybe also require some wrappers around the associated objects). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_113899
On Tue Aug 26 14:54:27 2025 +0000, Rémi Bernon wrote:
Well fwiw I'm not sure anymore about the thread safety, like you said it seems it is supposed to be guarded if we consider the Rust projection. OTOH the SDK headers also are apparently quite obviously unsafe at least in the IMap templated implementation. I checked again my tests and it's very possible that the crash I was able to trigger was caused by some overlook on my part (I had a thread wait timeout and perhaps it went past it, releasing the map while threads were still using it). Overall it looks like either PropertySet has a dedicated, safe, IMap implementation, or it adds a lock around every generic unsafe IMap calls (which maybe also require some wrappers around the associated objects). In that case, we can still use your implementation by guarding access to the entries with an SRWLOCK, right?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8685#note_113902
participants (5)
-
Nikolay Sivov (＠nsivov) -
Rémi Bernon
-
Vibhav Pant
-
Vibhav Pant (＠vibhavp)
-
Zhiyi Zhang (＠zhiyi)