{"id":104444,"date":"2020-11-12T07:00:00","date_gmt":"2020-11-12T15:00:00","guid":{"rendered":"https:\/\/devblogs.microsoft.com\/oldnewthing\/?p=104444"},"modified":"2025-02-07T19:53:43","modified_gmt":"2025-02-08T03:53:43","slug":"20201112-00","status":"publish","type":"post","link":"https:\/\/devblogs.microsoft.com\/oldnewthing\/20201112-00\/?p=104444","title":{"rendered":"Destructing outside the lock when removing items from C++ standard containers"},"content":{"rendered":"<p>Last time, we saw that <a href=\"https:\/\/devblogs.microsoft.com\/oldnewthing\/20201111-00\/?p=104439\"> object destruction is a hidden callout<\/a> that can cause you to violate the rule against calling out to external code while holding an internal lock. To recap, here&#8217;s the class we were using as our example:<\/p>\n<pre>class ThingManager\r\n{\r\nprivate:\r\n  std::mutex things_lock_;\r\n  std::vector&lt;std::shared_ptr&lt;Thing&gt;&gt; things_;\r\n\r\npublic:\r\n   void AddThing(std::shared_ptr&lt;Thing&gt; 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        [&amp;](auto&amp;&amp; thing)\r\n        {\r\n          return thing-&gt;id() == id;\r\n        });\r\n      if (it != things_.end()) {\r\n        things_.erase(it);\r\n      }\r\n   }\r\n};\r\n<\/pre>\n<p>We saw that there was a problem at the call to <code>things_.<\/code><code>erase()<\/code> because the erasure will result in the destruction of the <code>shared_ptr<\/code>, which could in turn trigger the destruction of an object you don&#8217;t control. And that uncontrolled code could try to call back into the <code>Thing\u00adManager<\/code>, resulting in a mess.<\/p>\n<p>Solving this problem can be tricky in general, although things are a bit easier for us in this case because we know some properties of <code>shared_ptr<\/code>, most importantly that it is noexcept movable.<\/p>\n<p>What we want to do is to remove the element from the vector without destructing it immediately. We want to extend the object&#8217;s lifetime until after the lock has been released.<\/p>\n<p>Here&#8217;s one way of doing it that works specifically for <code>shared_ptr<\/code>, taking advantage of its inexpensive default constructor.<\/p>\n<pre>void RemoveThingById(int32_t id)\r\n{\r\n  <span style=\"border: solid 1px currentcolor;\">std::shared_ptr&lt;Thing&gt; removed;<\/span>\r\n  std::lock_guard guard(things_lock_);\r\n  auto it = std::find_if(things_.begin(), things_.end(),\r\n    [&amp;](auto&amp;&amp; thing)\r\n    {\r\n      return thing-&gt;id() == id;\r\n    });\r\n  if (it != things_.end()) {\r\n    <span style=\"border: solid 1px currentcolor;\">removed = std::move(*it);<\/span>\r\n    things_.erase(it);\r\n  }\r\n}\r\n<\/pre>\n<p>Before entering the lock, we create an empty <code>shared_ptr<\/code> and use that to hold the element we intend to remove. C++ destructs objects in reverse order of construction, so declaring it before the <code>guard<\/code> means that the <code>removed<\/code> variable destructs after the guard is destructed; in other words, the <code>removed<\/code> variable destructs outside the lock.<\/p>\n<p>If the container is a <code>std::map<\/code> or <code>std::unordered_map<\/code>, you can accomplish this delayed destruction in general by using the <code>extract<\/code> method to extract the entire node, and then destruct the node outside the lock. For <code>std::list<\/code> and <code>std::forward_list<\/code>, you can <code>splice<\/code> the element out of the container into a temporary initially-empty list. But for containers like vectors, you&#8217;ll need to move the object out of the container, in which case you&#8217;re counting on the move operation not doing anything scary.<\/p>\n<p>Mind you, you&#8217;re already assuming that the move operation isn&#8217;t doing anything scary, because your vector mutation operations are going to move objects around, too.<\/p>\n<p>Fortunately, for things like <code>shared_ptr<\/code>, <code>unique_ptr<\/code>, and (in Windows) COM wrappers, the move operations are indeed not scary. They just shuffle pointers around. (I&#8217;m assuming you&#8217;re moving into an empty object. Obviously, if the destination of the move is not empty, then you&#8217;re going to run scary code when the previous contents of the destination are released.)<\/p>\n<p>Here are some ways of extracting elements from a container for later destruction:<\/p>\n<pre>\/\/ map, multimap, set, multiset,\r\n\/\/ and unordered versions of same\r\nauto removed = source.extract(it);\r\n\r\n\/\/ vector, dequeue\r\nauto removed = std::move(*it);\r\nsource.erase(*it);\r\n\r\n\/\/ list, forward_list\r\nstd::list&lt;T&gt; removed; \/\/ or std::forward_list\r\nremoved.splice(removed.begin(), source, it);\r\nreturn removed;\r\n<\/pre>\n<p>If you need to delay-destruct multiple items, you could extract them into a temporary vector.<\/p>\n<pre>template&lt;typename FwdIt, typename OutIt, typename Pr&gt;\r\nFwdIt extract_if(FwdIt first, const FwdIt last, OutIt out, Pr pred)\r\n{\r\n    first = std::find_if_not(first, last, std::ref(pred));\r\n    auto next = first;\r\n    for (; first != last; ++first) {\r\n            if (pred(*first)) {\r\n                *out = std::move(*first);\r\n                ++out;\r\n            } else {\r\n                *next = std::move(*first);\r\n                ++next;\r\n            }\r\n    }\r\n\r\n    return next;\r\n}\r\n\r\nstd::vector&lt;T&gt; removed;\r\nv.erase(extract_if(v.begin(), v.end(), std::back_inserter(removed), filter), v.end());\r\n<\/pre>\n<p>Keep the <code>removed<\/code> object alive beyond the release of the lock, so that the objects are destructed outside the lock.<\/p>\n<p><b>Bonus chatter<\/b>: It would be convenient if <code>vector<\/code>, <code>list<\/code>, and <code>forward_list<\/code> also had <code>extract<\/code> methods which returned the removed object, similar to the node extractors for the map and set types. Until then, we&#8217;ll have to write our own:<\/p>\n<pre>template&lt;typename T&gt;\r\nT extract(\r\n    std::vector&lt;T&gt;&amp; v,\r\n    typename std::vector&lt;T&gt;::iterator it)\r\n{\r\n    auto cleanup = wil::scope_exit([&amp;] { v.erase(it); });\r\n    return std::move(*it);\r\n}\r\n\r\ntemplate&lt;typename T&gt;\r\nstd::list&lt;T&gt; extract(\r\n    std::list&lt;T&gt;&amp; source,\r\n    typename std::list&lt;T&gt;::const_iterator it)\r\n{\r\n    std::list&lt;T&gt; removed;\r\n    removed.splice(removed.begin(), source, it);\r\n    return removed;\r\n}\r\n\r\ntemplate&lt;typename T&gt;\r\nstd::forward_list&lt;T&gt; extract_after(\r\n    std::forward_list&lt;T&gt;&amp; source,\r\n    typename std::forward_list&lt;T&gt;::const_iterator it)\r\n{\r\n    std::forward_list&lt;T&gt; removed;\r\n    removed.splice_after(removed.before_begin(), source, it);\r\n    return removed;\r\n}\r\n<\/pre>\n<p><b>Exercise<\/b>: Why did I use a <code>scope_exit<\/code> in the vector extractor? Why not<\/p>\n<pre>template&lt;typename T&gt;\r\nT extract(std::vector&lt;T&gt;&amp; v,\r\n    std::vector&lt;T&gt;::iterator it)\r\n{\r\n    auto result = std::move(*it);\r\n    v.erase(it);\r\n    return result;\r\n}\r\n<\/pre>\n<p><b>Update<\/b>: Wrote <code>extract_if<\/code> because the previous version used <code>remove_if<\/code> incorrectly.<\/p>\n","protected":false},"excerpt":{"rendered":"<p>Managing the order of destruction.<\/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-104444","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"<p>Managing the order of destruction.<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/104444","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=104444"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/104444\/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=104444"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=104444"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=104444"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}