This is a general problem, but I’m going to give a specific example.<\/p>\n
C++ standard library collections do not support concurrent mutation. It is the caller’s responsibility to serialize mutation operations, and to ensure that no mutation occurs at the same time as any other operation. And the usual way of accomplishing this is to use a mutex of some kind.<\/p>\n
class ThingManager\r\n{\r\nprivate:\r\n std::mutex things_lock_;\r\n std::vector<std::shared_ptr<Thing>> things_;\r\n\r\npublic:\r\n void AddThing(std::shared_ptr<Thing> thing)\r\n {\r\n std::lock_guard guard(things_lock_);\r\n things_.push_back(std::move(thing));\r\n }\r\n\r\n void RemoveThingById(int32_t id)\r\n {\r\n std::lock_guard guard(things_lock_);\r\n auto it = std::find_if(things_.begin(), things_.end(),\r\n [&](auto&& thing)\r\n {\r\n return thing->id() == id;\r\n });\r\n if (it != things_.end()) {\r\n things_.erase(it);\r\n }\r\n }\r\n};\r\n<\/pre>\nThe idea here is that you give the Thing\u00adManager<\/code> a bunch of things, and then you can later remove them by providing the Thing<\/code>‘s ID. Presumably there are also methods to search for Thing<\/code>s or to perform some operation across all Thing<\/code>s, but those are just distractions from the exercise.<\/p>\nThis particular object wants to support concurrent operation, so it internally uses a mutex to ensure safe operation.<\/p>\n
Now, you can quibble about the use of find_if<\/code> instead of remove_if<\/code>, or using a std::vector<\/code> instead of a std::map<\/code>, but let’s set that aside.<\/p>\nThe question is: What’s wrong with this code?<\/p>\n
I sort of gave it away in the title: We are calling out to external code while holding our lock.<\/p>\n
You probably know not to call out to external code when holding an internal lock, and the act of invoking a method on an object may remind you of that fact. But destructors just run by themselves. You don’t typically write code the trigger the destruction of an object. The object just destructs when it destructs.<\/p>\n
Removing the shared_ptr<Thing><\/code> from our vector could result in the Thing<\/code>‘s destruction if this was the last strong reference to the Thing<\/code>. And that destructor runs while the things_lock_<\/code> is still locked.<\/p>\nNow things get interesting.<\/p>\n
You may not know all that happens inside the Thing<\/code> destructor. It may have been written by another team, or by you, six months ago. Or somebody could have derived from Thing<\/code> and given you a shared pointer to the derived object.\u00b9 Or you might be given a shared pointer to a Thing<\/code> embedded inside a larger object.<\/p>\nLet’s do that thing with the derived type:<\/p>\n
class SuperThing : Thing\r\n{\r\nprivate:\r\n ThingManager& manager_;\r\n int32_t helper_id_ = 0;\r\n\r\npublic:\r\n SuperThing(ThingManager& manager) :\r\n manager_(manager)\r\n {\r\n auto helper = std::make_shared<Thing>();\r\n helper_id_ = helper->id();\r\n manager_.AddThing(helper);\r\n }\r\n\r\n ~SuperThing()\r\n {\r\n manager_.RemoveThingById(helper_id_);\r\n }\r\n};\r\n<\/pre>\nThe Super\u00adThing<\/code> object is itself a Thing<\/code>, but it also uses a helper thing. At construction, it creates a helper thing and registers it with the manager, retaining the ID. And at destruction, it removes its helper thing.<\/p>\nAnd then this happens:<\/p>\n
void test(ThingManager& manager)\r\n{\r\n auto s = std::make_shared<SuperThing>();\r\n auto id = s->id();\r\n manager.AddThing(s);\r\n s = nullptr;\r\n\r\n manager.RemoveThingById(id);\r\n}\r\n<\/pre>\nThis little test function creates a Super\u00adThing<\/code>, adds it to the thing manager, and then immediately removes it.<\/p>\nThe Remove\u00adThing\u00adBy\u00adId<\/code> function looks for a matching Id and finds it, so it erases the corresponding Thing<\/code> from the vector. That erasure destroys the shared_<\/code>ptr<\/code>, and since this is the last strong reference, the underlying Thing<\/code> is also destroyed.<\/p>\nThis runs the destructor of our Super\u00adThing<\/code>, which tries to remove its helper Thing<\/code>. And that calls back into the ThingManager<\/code>, which gets stuck trying to acquire a mutex that is already held (unwittingly, by itself).<\/p>\nThis is not a purely theoretical exercise. This sort of thing happens, and it’s a source of bugs.<\/p>\n
Next time, we’ll look at how to address these types of problems.<\/p>\n
\u00b9 If you work in Windows, a common scenario for this is that the shared_ptr<\/code> in the example above takes the form of a COM reference-counted pointer.<\/p>\n","protected":false},"excerpt":{"rendered":"You may not know where that’s going to lead.<\/p>\n","protected":false},"author":1069,"featured_media":111744,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[1],"tags":[25],"class_list":["post-104439","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
You may not know where that’s going to lead.<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/104439","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/users\/1069"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/comments?post=104439"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/104439\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media\/111744"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media?parent=104439"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=104439"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=104439"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}