A team asked me to review their pull request that fixed a bug that was caused by failing to perform some required action along all code paths. Here’s a simplified sketch:
void MySpecialFeature::OnButtonClick()
{
try {
auto file = PickFile();
if (!file) {
DismissUI();
return;
}
if (ConfirmAction()) {
if (m_useAlgorithm1) {
// StartAlgorithm1 invokes the lambda when finished.
StartAlgorithm1(file, [self = shared_from_this()] {
self->DismissUI();
});
} else {
RunAlgorithm2(file);
DismissUI();
}
} else { // this block was missing
 DismissUI();
}
} catch (...) {
DismissUI();
}
}
The problem was the DismissUI() call was missing on one of the code paths.
I suggested that they simplify the code by using an RAII type which will dismiss the UI on all code paths automatically.
The Windows Implementation Library (commonly known as wil) provides a helper called scope_exit which creates and returns an RAII object whose destructor runs the lambda you specify.¹ We can tell it to run a lambda that calls DismissUI().
Here’s the first try:
void MySpecialFeature::OnButtonClick()
{
auto ensureDismiss = wil::scope_exit([&] { DismissUI(); });
try {
auto file = PickFile();
if (!file) {
// DismissUI();
return;
}
if (ConfirmAction()) {
if (m_useAlgorithm1) {
// StartAlgorithm1 invokes the lambda when finished.
StartAlgorithm1(file, [self = shared_from_this()] {
???
});
} else {
RunAlgorithm2(file);
// DismissUI();
}
} // else {
 // DismissUI();
// }
} catch (...) {
// DismissUI();
}
}
The ensureDismiss is an RAII type created by scope_exit that runs the lambda at destruction, which in this case means that it calls DismissUI(). The nice thing about C++ destruction of local variables is that it happens when scope ends, whether it be by executing off the end of the block, by an early exit (say, an early return), or an exception.
There is one section marked with question marks, though. We don’t want DismissUI() to happen until Algorithm1 finishes its work. How do we delay the destruction of the ensureDismiss object?
Well, we can’t. It will destruct when the scope ends.
However, the objects created by wil:: are movable. When you move them, the obligation to run the lambda is transferred to the moved-to object. So you can move-capture the ensureDismiss into the lambda, and that will transfer the responsibility to call DismissUI() into the lambda. Fortunately, the lambda already captures shared_from_this(), so there is a shared_ptr<MySpecialFeature> inside the lambda. We can’t see it, but it’s clear that StartAlgorithm1 must move its lambda to some safekeeping location so that it can invoke it later. When the lambda moves, the obligation moves with it, and when the final moved-to lambda destructs, the DismissUI() will happen.
void MySpecialFeature::OnButtonClick()
{
auto ensureDismiss = wil::scope_exit([=] { DismissUI(); });
try {
auto folder = PickOutputFolder();
if (!folder) {
return;
}
if (ConfirmAction()) {
if (m_useAlgorithm1) {
// StartAlgorithm1 invokes the lambda when finished.
StartAlgorithm1(file, [self = shared_from_this(),
ensureDismiss = std::move(ensureDismiss)]
{ });
} else {
RunAlgorithm2(file);
}
}
} catch (...) {
}
}
Note that we changed the capture of the lambda passed to scope_exit a value capture, because a reference capture would be capturing references to locals that have destructed. In this case, we don’t capture any locals, so it’s not significant, but it’s good hygiene and avoids embarrasing questions later. (A hidden gotcha is that a value capture captures this by value, so there is actually still a reference capture in there, namely a reference to *this.)
In fact, for even better hygiene, you might want to capture the shared_ptr into the lambda so that the lifetime promises are more explicit.
void MySpecialFeature::OnButtonClick()
{
auto ensureDismiss = wil::scope_exit([self = shared_from_this()]
{ self->DismissUI(); });
try {
auto folder = PickOutputFolder();
if (!folder) {
return;
}
if (ConfirmAction()) {
if (m_useAlgorithm1) {
// StartAlgorithm1 invokes the lambda when finished.
StartAlgorithm1(file, [// self = shared_from_this(),
ensureDismiss = std::move(ensureDismiss)]
{ });
} else {
RunAlgorithm2(file);
}
}
} catch (...) {
}
}
There’s a little wrinkle here that we can iron out look at next time.
Bonus chatter: The object created by wil:: patterns itself after std::unique_ptr, so you can reset() it to go through its destructor cleanup early, and you can release() it to say “I changed my mind. Don’t run the lambda at all.”
¹ A corresponding proposal for the C++ standard library has been around since 2013. (Followed up by N3830, N3949, N4189, p0052r1, through p0052r10, then became part of N4786 (Working Draft, C++ Extensions for Library Fundamentals, Version 3), which appears to have been approved in 2024 as an experimental feature.
AIUI, Algorithm1 doesn’t actually promise to get rid of the lambda when it’s done, and could theoretically hold on to (a moved copy of) it for much longer. So the lambda should probably be
[ensureDismiss = std::move(ensureDismiss)] { ensureDismiss.reset(); }Seems to be exactly the topic of the very next blog post.
This issue could also be solved by adding a flag to the `scope_exit` object and only running the lambda if the flag is engaged. This can be accessed through methods like `dismiss()` and `rehire()`.
Re: Why is Windows still tinkering with critical sections?
Since I can't comment in that post anymore I'll post it here. Sorry for the off-topicness but it needs to be said.
Sure we can appreciate that the Windows development team makes improvements to the products they make, I'd imagine that having to maintain such an old and bloated codebase is no easy task, but then again, that's the reason Windows still exists in the first place. No other platform offers the backwards compatibility Windows offers, and I believe that's why a lot of companies and individuals around the world still use it...
That can only go so far, and third party developers need to meet halfway.
It is for the best that Microsoft tests if stuff breaks, and tries as hard as they can to avoid breakage, but the problem YOU don't seem to realize is that a lot of that stuff is ALREADY broken.
Like that bug in GTA, because of the Critical Section change?
NOTHING that is documented as part of critical sections has changed. If you explicitly stuck to the API contract of how you were supposed to use critical sections, your code would not break.
The internals of how critical sections operate...
I think a recent C++ standard actually deprecated the behavior of `[=]` capturing `this`, it generates a compiler warning under MSVC when compiling with the latest language standards. Whenever lambda lifetime is questionable I avoid the automatic captures entirely and stick only to explicit capture.
An annoyance with running actions on scope exit is the potential for exceptions terminating the application, there's not really a one-size-fits-all solution for that. The best you could do is check whether the scope is exiting due to an exception and invoke the scope exit action in a try block, discarding an exceptions, otherwise invoke it...
Whether stack unwinding is done before calling std::terminate upon unhandled exception, is implementation-defined. By default, none of MSVC, GCC, Clang unwinds stack in this case. Throwing destructors are another level of craziness.
Luckily in the specific case of the post, dismissing UI is so many orders of lesser importance to worry about when the process is being fried. If the UI is in-process, it goes away sadly (happily?) together. If the UI is in another process, this other process of course should be ready for the controlling process to be suddenly gone.
(This is in reply to LB's comment in GL's comment in LB's top-level comment, in case the commenting system gets crazy again.)
Apparently I read
>An annoyance with running actions on scope exit is the potential for exceptions terminating the application
differently than you intended. I initially read "exception terminating app" as one thrown prior to entering the dtor of scope_exit object (the lambda), and "no one-size-fits-all" as "whether you want to do something different upon an (unhandled) exception is case-by-case", then the ensuing discussion suggested putting some detection logic in lambda (inside dtor), hence mentioning the dtor might not even be...
I didn’t mention anything about stack unwinding for uncaught exceptions or std::terminate so I don’t know why you brought that up. I’m just talking about how DismissUI could throw, and how using an RAII wrapper for it could result in unexpected or difficult to remedy behavior changes.