gsl::finally executes throwing code under noexcept, resulting in runtime crash

[apenn-msft]

gsl::finally in name, API, and documentation purports to allow arbitrary code to be executed at scope exit, however in practice it executes the provided function within its noexcept(true) destructor which results in a runtime crash if the provided action throws.

In name, there is an implied understanding for gsl::finally, which borrows the "finally" concept from C-based languages, to execute arbitrary, even throwing code. This behavior models what "finally" means in other C-based languages where there isn't a constraint on code in the finally block being non-throwing.

In API, gsl::finally will accept a throwing action without indication/enforcement that the action must not throw even though in practice and at runtime, this is a requirement otherwise the program terminates:

gsl::finally([] { throw 1; }); // valid code which compiles and crashes at runtime

In documentation, it documents that "something gets run", implying any arbitrary code is fine.

`final_action` allows you to ensure something gets run at the end of a scope.

In summary, programmers do want a utility to help execute code at scope exit.
First we must decide if we should provide such a utility or if not, and what alternative should exist if any to make code such as:

if (...) Update(); return;
if (...) Update(); return;
if (...) Update(); return;

which would be a reason a programmer would seek out gsl::finally as a utility to execute Update prior to returning without having to manually write it out n-many times.

Next, we must decide if we place exception constraints on gsl::finally.
We may decide to truly allow arbitrary code to execute in gsl::finally, in which case we could specify its destructor's noexcept(false) or based on a template parameter or whether the provided action throws.
If we instead decide to retain gsl::finally's noexcept execution of the action, we should decide how to solve for the problem of:

gsl::finally([] { throw 1; };

where we know at compile time such code is invalid and will terminate at runtime. Can we enforce this at compile time? Yes, i.e. by requiring std::is_nothrow_invocable on the function.

Finally, in addition to compile time enforcement, the documentation (and possibly the name) should be updated to make it clear that gsl::finally can only be used with non-throwing code and the user must ensure the supplied action must not throw (else the program will terminate)

[apenn-msft]

proposed solution for these points here:
#1192
#1191
#1190

[apenn-msft]

interestingly this has probably been discussed before, e.g.
https://github.com/ricab/scope_guard

A public, general, simple, and fast C++11 scope guard that defends against implicitly ignored returns and optionally enforces noexcept at compile time (in C++17), all in a SFINAE-friendly maner.

see
https://github.com/ricab/scope_guard/blob/main/docs/interface.md#compilation-option-sg_require_noexcept_in_cpp17
and
https://github.com/ricab/scope_guard/blob/main/docs/design.md#implications-of-requiring-noexcept-callbacks-at-compile-time

[apenn-msft]

another implementation enforcing noexcept
https://www.lukas-barth.net/blog/scope-guards-resource-leaks/

template<typename T>
concept CallableInScopeGuard = requires(const T& callable) {
  // The callable needs to be callable with no arguments, be noexcept, and
  // should return void since we're discarding the return value.
  { callable() } noexcept -> std::same_as<void>;
};

template<CallableInScopeGuard Func>
class ScopeGuard {
  // …
private:
  Func storedFunc;
};

[apenn-msft]

and a proposal which suggests allowing noexcept(true/false) depending on the provided action:
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4189.pdf

 // release
 ~scope_exit() noexcept(noexcept(this->exit_function())){
 if (execute_on_destruction)
 this->exit_function()

[apenn-msft]

more caveats in research pointing out unsurprisingly there are many interpretations of a scope guard and they can be confusing:
https://bajamircea.github.io/coding/cpp/2018/04/12/scope-guard.html

However scope guards have issues:

Yet another idiom with subtle behaviours around exception handling that users need to understand (in particular which are the choices made by the specific scope guards used)

[apenn-msft]

https://en.cppreference.com/w/cpp/experimental/scope_success/%7Escope_success

~scope_success() noexcept(noexcept(std::declval<EF&>()()));
(library fundamentals TS v3)
Calls the exit function if the result of std::uncaught_exceptions() is less than or equal to the counter of uncaught exceptions (typically on normal exit) and the scope_success is active, then destroys the stored EF (if it is a function object) and any other non-static data members.

[carsonRadtke]

Thank you for reporting this issue! This will be discussed at our monthly maintainer's sync. Unfortunately, GSL does not have the power to change the semantics of gsl::finally without the isocpp/CppCoreGuidelines authors modifying the specification. To get a change in GSL we may need to start with an amendment to the guidelines. @gdr-at-ms and I will discuss all of this and update this issue.

We appreciate you taking the time to explore alternative implementations for a finally-like machine. We will evaluate these solutions and determine which is best for GSL; then we can work on adopting into the guidelines and implementing in Microsoft/GSL.