On Mon Jul 3 20:02:17 2023 +0000, Rémi Bernon wrote:
Actually I think I missed the case where multiple threads are calling IWeakReference_Resolve concurrently. My suggestion is not behaving correctly there, as increasing public refcount by 1 may make other threads to succeed. Maybe a separate "invalid" field would be better but I need to think about it. I suggest to leave this aside for now and only include the changes that do not touch weak references yet, I think they look good now.
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.