The COM static store, part 2: Race conditions in setting a singleton
Last time, we learned about the COM static store and how you can use it to manage objects whose lifetime should be tied to the lifetime of COM, rather than to the lifetime of the process. And I noted that there were some loose ends that needed to be tidied.
If two threads both try to access the singleton at the same time, there’s a race condition where both of them see that there is no object in the cache, so both threads create an object, put it in the cache, and return it. This means that one thread’s object is actually in the cache, and the other thread’s object is just hanging around. If the object has any state, then the losing thread will mutate the object state, but those changes won’t be visible to anyone else.
To avoid this problem, we need a lock. Let’s try this:
Thing GetSingletonThing()
{
auto props = CoreApplication::Properties();
if (auto found = props.TryLookup(L"Thing")) {
return found.as<Thing>();
}
auto thing = MakeAThing();
auto guard = std::lock_guard(m_lock);
if (auto found = props.TryLookup(L"Thing")) {
return found.as<Thing>();
}
props.Insert(L"Thing", thing);
return thing;
}
If we don’t see an object in the cache, we create a fresh object, but before putting it into the cache, we check if another thread beat us to it. If so, then we throw away the one we had created and switch to the one that’s already in the cache.
If there is still nothing in the cache, then our object is the one that goes into the cache.
This algorithm assumes that creating a Thing and throwing it away is relatively harmless. If creating a Thing has side effects that prevent multiple instances,¹ or if it entails excessive computation or other resources requirements, you may not want to create one if there’s a chance you’re just going to throw it away.
We’ll try to solve that problem next time.
Bonus chatter: May as well take this time to make the function more reusable.
template<typename T, typename Maker>
T GetSingleton(std::wstring_view name, Maker&& maker)
{
auto props = CoreApplication::Properties();
if (auto found = props.TryLookup(name)) {
return winrt::unbox_value<T>(found);
}
auto value = Maker();
static winrt::slim_mutex lock;
winrt::slim_lock_guard const guard{ lock };
found = props.TryLookup(name);
if (auto found = props.TryLookup(name)) {
return winrt::unbox_value<T>(found);
}
props.Insert(name, winrt::box_value(value));
return value;
}
We let the caller pass an arbitrary functor for making the object. The name is also a runtime parameter rather than a compile-time (template) parameter, so that you can generate unique names at runtime.
Using box_value and unbox_value allows us to store both Windows Runtime reference types and value types in the property set.
¹ For example, the object may, at construction, register with some other component, and that other component supports only one client at a time. Or the object, at construction, may attempt to access some resource that cannot be shared, like a file in exclusive mode.
Light
Dark
8 comments
Two points that weren’t obvious to me:
* The use of a lock isn’t COM-specific; you needed synchronization primitives back in 2011 for non-COM global singletons.
* The COM store is for more than singletons, so it supports overwriting stored objects (I think?). The problem with the original code isn’t so much the race; it’s that
might overwrite the singleton you already stashed away. Otherwise, you could just make sure to return whatever’s in the store following a (possibly no-op) write.
Is this intentional or is it a copy & pasta bug?
found = props.TryLookup(name);
if (auto found = props.TryLookup(name)) {
I’m afraid the
GetSingletonfunction is not correct, because the function is templated overMaker, which means each instantiation of the function template will have its ownlockstatic variable (z/oe8YnT). If two sites of invocation pass in the samenameand two different lambdas, the accesses will not be serialized.I thought the mangled name of Maker would be the same for different lambdas with the same “signature”, but it seems that the mangled names of two different lambda variables of what I thought of as the same type (“function” signature) are not the same and thus the statics (addressof) in GetSingleton (for example) are not the same. Probably for good reasons…
Passing the exact same lambda variable works as expected as you say.
Jason Turner has excellent videos on lambdas on YouTube, I highly recommend checking them out
I don’t think this is all safe. What if I have a DLL with a COM class and I want to store a singleton from this DLL. This DLL is then unloaded, but the object is stuck in some global application storage and contains vtable pointers to code segments, that are no longer in memory. It is quite common that COM objects are in separate libraries and COM does unload them if DllCanUnloadNow returns TRUE. I guess that you would have to implement such function as to simply return false. Could I just use static storage in my DLL? Am I guaranteed, that my class DLL is unloaded before COM shuts down? If no, than I think the best approach is to have some kind of reference counted global state and store a weak pointer in DLL’s static storage and just get reference to the object (or create a new one) when class factory is created and store it in class factory and let the factory pass the reference to created objects. And the global state will be simply released when last object is destroyed. Of course this would add overhead of creating the global state again and again if all your objects are destroyed and then new ones are created first.
If
DllCanUnloadNowreturnsTRUEwhile there are still living objects referencing code in that DLL, I’ll consider it a brokenDllCanUnloadNow.COM is still basically the very same animal it was ~30 years ago, so I don’t know why you came to ask such basic questions here. It’s still just COM.