Say you want to perform a bunch of asynchronous operations involving some object state, but also want to make sure that no other tasks access that object state at the same time. For synchronous code, you would use a traditional synchronization object like a mutex or critical section:<\/p>\n
void MyObject::RunOne()\r\n{\r\n std::lock_guard guard(m_mutex);\r\n\r\n if (!m_list.empty()) {\r\n auto& item = m_list.front();\r\n item.Run();\r\n item.Cleanup();\r\n m_list.pop_front();\r\n }\r\n}\r\n<\/pre>\nThe mutex ensures that only one attempt to process an item from the list is active at a time, and also to prevent any other code from mutating the m_list<\/code> while we are using it.<\/p>\nBut say that some of these operations are asynchronous. For simplicity, I’m eliding the traditional auto lifetime = get_strong();<\/code> that is used to prevent the object from being destructed while awaiting. (Let’s say that the rule is that you cannot release your reference to MyObject<\/code> until Run\u00adOne\u00adAsync<\/code> completes.)<\/p>\nIAsyncAction MyObject::RunOneAsync()\r\n{\r\n std::lock_guard guard(m_mutex);\r\n\r\n if (!m_list.empty()) {\r\n auto& item = m_list.front();\r\n co_await item.RunAsync();<\/span>\r\n item.Cleanup();\r\n m_list.pop_front();\r\n }\r\n}\r\n<\/pre>\nIs this okay?<\/p>\n
One argument I’ve heard is that this is not okay because the co_await<\/code> causes the original Run\u00adOne\u00adAsync<\/code> call to to return an IAsync\u00adAction<\/code> to its caller, and as part of the act of returning the IAsync\u00adAction<\/code>, the lock is released.<\/p>\nThis argument is incorrect. The lock remains held while the coroutine is suspended. After all, if objects were destructed at suspension, then you wouldn’t be able to carry anything across a suspension point!<\/p>\n
IAsyncAction WidgetManager::WhateverAsync()\r\n{\r\n auto lifetime = get_strong();\r\n std::string name = m_widget.GetName();\r\n m_widget.SetName(\"temporary\");\r\n co_await m_widget.SomethingAsync();\r\n m_widget.SetName(name); \/\/ certainly \"name\" is still valid, right?\r\n \/\/ certainly \"lifetime\" is still holding our object alive, right?\r\n}\r\n<\/pre>\nDon’t worry. name<\/code> and lifetime<\/code> are still valid across the suspension because the formal parameters and local variables are kept in the coroutine frame, which remains alive while the coroutine is suspended. Indeed, the lifetime<\/code> relies upon it!<\/p>\nHowever, it’s the liveness of the lock guard that is the issue here.<\/p>\n
Since the lock guard hasn’t been destructed, the mutex remains locked while the coroutine is suspended.<\/p>\n
Now things get exciting.<\/p>\n
Suppose we have another coroutine that wants the lock. Heck, it could very well be another call to Run\u00adOne\u00adAsync<\/code>!<\/p>\n\n\n\nRunOneAsync<\/code> #1<\/td>\n<\/tr>\n\n\u00a0<\/td>\n construct lock_guard<\/td>\n <\/td>\n m_mutex.lock()<\/code><\/td>\n<\/tr>\n\n\u00a0<\/td>\n auto& item = m_list.front();<\/code><\/td>\n<\/tr>\n\n\u00a0<\/td>\n co_await item.RunAsync();<\/code><\/td>\n\u2192<\/td>\n Suspended<\/td>\n<\/tr>\n \nRunOneAsync<\/code> #1 returns IAsyncAction<\/code><\/td>\n<\/tr>\n\n\u2193<\/td>\n<\/tr>\n \nThread available to do other work<\/td>\n<\/tr>\n \n\u2193<\/td>\n<\/tr>\n \nRunOneAsync<\/code> #2<\/td>\n<\/tr>\n\n\u00a0<\/td>\n construct lock_guard<\/td>\n <\/td>\n m_mutex.lock()<\/code><\/td>\n<\/tr>\n<\/tbody>\n<\/table>\nNow we’re in trouble.<\/p>\n
If the m_mutex<\/code> supports recursive acquisition, then what happens is that the second call to Run\u00adOne\u00adAsync<\/code> successfully acquires the mutex (recursive acquisition), and execution continues:<\/p>\n\n\n\n\u00a0<\/td>\n RunOneAsync<\/code> #2 continues<\/td>\n<\/tr>\n\n\u00a0<\/td>\n auto& item = m_list.front();<\/code><\/td>\n<\/tr>\n\n\u00a0<\/td>\n co_await item.RunAsync();<\/code><\/td>\n\u2192<\/td>\n Suspended<\/td>\n<\/tr>\n \nRunOneAsync<\/code> #2 returns IAsyncAction<\/code><\/td>\n<\/tr>\n\n\u2193<\/td>\n<\/tr>\n \nThread available to do other work<\/td>\n<\/tr>\n<\/tbody>\n<\/table>\nWe are running the front element twice! I bet it’s not expecting that.<\/p>\n
The mutex failed at its intended purpose of serializing calls to Run\u00adOne\u00adAsync<\/code>.<\/p>\nOkay, but wait, the disaster is still unfolding.<\/p>\n
Eventually, the two calls will complete, in some order. Let’s say that #1 finishes first. Execution continues:<\/p>\n
\n\n\nRunOneAsync<\/code> #1 resumes<\/td>\n\u2190<\/td>\n RunAsync<\/code> #1 completes<\/td>\n<\/tr>\n\n\u00a0<\/td>\n item.Cleanup();<\/code><\/td>\n <\/td>\n Cleaning up while RunAsync<\/code> #2 still outstanding<\/td>\n<\/tr>\n\n\u00a0<\/td>\n m_list.pop_front();<\/code><\/td>\n <\/td>\n Destructing head item while #2 is still using it<\/td>\n<\/tr>\n \n\u00a0<\/td>\n destruct lock_guard<\/td>\n <\/td>\n m_mutex.unlock()<\/code><\/td>\n<\/tr>\n\nRunOneAsync<\/code> #1 completes<\/td>\n<\/tr>\n\n\u2193<\/td>\n<\/tr>\n \nThread available to do other work<\/td>\n<\/tr>\n \n\u2193<\/td>\n<\/tr>\n \nRunOneAsync<\/code> #2 resumes<\/td>\n\u2190<\/td>\n RunAsync<\/code> #2 completes<\/td>\n<\/tr>\n\n\u00a0<\/td>\n item.Cleanup();<\/code><\/td>\n <\/td>\n Cleaning up already-destructed object<\/td>\n<\/tr>\n \n\u00a0<\/td>\n m_list.pop_front();<\/code><\/td>\n <\/td>\n Popping an item that was never run<\/td>\n<\/tr>\n \n\u00a0<\/td>\n destruct lock_guard<\/td>\n <\/td>\n m_mutex.unlock()<\/code><\/td>\n<\/tr>\n\nRunOneAsync<\/code> #2 completes<\/td>\n<\/tr>\n<\/tbody>\n<\/table>\nWhen the first RunAsync<\/code> completes, the first Run\u00adOne\u00adAsync<\/code> resumes, and it proceeds to clean up the item that finished running, and then remove the head item from the list, thereby destructing it. All this happens even though the second Run\u00adOne\u00adAsync<\/code> is still using it.<\/p>\nThe first Run\u00adOne\u00adAsync<\/code> completes, having created a right mess of things but escaping unharmed.<\/p>\nWhen the second Run\u00adAsync<\/code> completes, the second Run\u00adOne\u00adAsync<\/code> resumes, and it tries to clean up the item that has already been destructed. You get sent this crash dump and you scratch your head because you’re looking at the code and you see that mutex right there, and you’re thinking, “How can this thing get prematurely destructed? It’s protected by a mutex!”<\/p>\nNow, maybe the Cleanup<\/code> method happens by sheer luck not to crash. It “only” corrupts some memory. That just makes the debugging even harder.<\/p>\nThe second Run\u00adOne\u00adAsync<\/code> then pops the front item from the list, thinking it’s popping the item that it just finished running, when in fact it’s popping an item on the list that was never run at all<\/i>.<\/p>\nNow the bug is that the program keeps running, but sometimes, items put onto the work list are thrown away without ever being run or cleaned up. Meanwhile, some items are run twice. This bug doesn’t come with crash dumps. It’s just end-user reports from the field that your program isn’t doing its job.<\/p>\n
Basically, what’s going on is that thanks to coroutines sharing a thread, your recursive mutex is not doing its job of ensuring mutual exclusion. Since everything is happening on a single thread, the recursive mutex always says, “Oh, I remember you. Come on in!”<\/p>\n
Next time, we’ll look at what happens if the mutex does not support recursive acquisition.<\/p>\n","protected":false},"excerpt":{"rendered":"
One way things can go wrong.<\/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-105417","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
One way things can go wrong.<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/105417","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=105417"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/105417\/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=105417"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=105417"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=105417"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}