Re: [PATCH 3/5] comsvcs: Implement ISharedPropertyGroupManager_CreatePropertyGroup().

[Nikolay Sivov]

On 8/17/20 3:54 PM, Jactry Zeng wrote:
> - if (!refcount)
> - {
> -
>    struct group_manager *manager = impl_from_ISharedPropertyGroupManager(group->parent);
> -
>    size_t count, index;
> -
> -
>    SysFreeString(group->name);
> -
> -
>    index = group - manager->property_groups;
> -
>    manager->count--;
> -
>    count = manager->count - index;
> -
>    if (count)
> -
>        memmove(&manager->property_groups[index], &manager->property_groups[index + 1], count * sizeof(*manager->property_groups));
> -
> -
>    ISharedPropertyGroupManager_Release(group->parent);
> - }

That's not ideal to have back link to manager like that, but I see tests
show that it might be happening. It's probably easier to keep a list
instead, you can then unlink and release parent, potentially with some
locking later. You can also store 'struct group_manager*' directly
there, so you don't have to do an impl_from_*().
> - name = SysAllocString(L"testgroupname2");
> - isolation = 0;
> - release = 1;
> - exists = TRUE;
> - expected_refcount = get_refcount(manager) + 1;
> - hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group1);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - ok(!exists, "Got unexpected value %d.\n", exists);
> - refcount = get_refcount(group1);
> - todo_wine ok(refcount == 2, "Got unexpected refcount: %d.\n", refcount);
> - refcount = get_refcount(manager);
> - ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
> - SysFreeString(name);

That's a weird bit. It might be important to try different release modes
to see what's going on.