On Mon Jul 3 20:37:17 2023 +0000, Fabian Maurer wrote:
What about something like this?
for (;;) { ref = impl->ref_public; if (!ref) { *out = NULL; return S_OK; } /* Only increment when it didn't change since last check */ ref2 = InterlockedCompareExchange(&impl->ref_public, ref + 1, ref); /* Value change since last check, try again */ if (ref2 != ref) continue; hr = IGeolocator_QueryInterface(&impl->IGeolocator_iface, iid, (void **)out); InterlockedDecrement(&impl->ref_public); return hr; }This should catch the case where another thread decrements after our check (since the compare exchange fails), the increment only happens when the ref really isn't 0.
Yes, that should work. I'd avoid unnecessary local variables and compare `InterlockedCompareExchange` result with ref directly. You should also probably do:
`if (!(ref = InterlockedOr(&impl->ref_public, 0)))`
Instead of reading the variable, as it'd otherwise not be an atomic operation.
Note that ideally a weak reference should not own the object data, and the object should be freed as soon as the last public reference is released. As we're implementing it on the same object here it is not possible to release the memory separately, but it's perhaps something to keep in mind.
Especially, if the object keeps any reference to other objects, it should release them in the public `geolocator_Release`, not in `weak_reference_Release`. Only freeing the memory would be delayed.