Hi,

On 02/02/2023 11:59, Frederick Virchanza Gotham via Std-Proposals wrote:
> This week I had to fix a bug in a desktop GUI program. After doing
> some testing, I was leaning toward the possibility that one of the
> functions in the program was being re-entered (when it shouldn't be).
> For a quick solution to test my theory, I protected the function with
> a mutex as follows:
>
> int Func(void)
> {
> static mutex mtx;
> lock_guard<mutex> mylock;
>
> // The rest of the function goes here
> }
>
> After making this change, the program locked up completely. I was a
> little puzzled so I changed the 'mutex' to a 'recursive_mutex', and
> then it no longer locked up, but the original bug was still presenting
> itself.
>
> So I figured that this function is being re-entered by the same
> thread. This was because the function was an event handler for the
> GUI, and the GUI sometimes created nested event loops, which meant
> that an event handler could be re-entered by the same thread (for
> example if you displayed a modal dialog box inside a event handler).
>
> So I needed to protect this function from re-entry, not just by
> multiple threads but also by the same thread. It would be nice if
> there were a core-language feature for this, for example:
>
> int Func(void) noreentry(-1)
> {
> // The rest of the function goes here
> }
>
> The idea here is that the function would immediately return -1 if you
> tried to re-enter it. As 'Func' is a normal function (i.e. not a
> member function), the prevention of reentry would apply to the entire
> program.
>
> If it were a member function, for example:
>
> struct MyClass {
> int MemberFunc(void) noreentry_this_object(-1)
> {
> // The rest of the function goes here
> }
> };
>
> then the prevention of reentry only applies to the current object (not
> to all objects). Alternatively, if you want to ensure prevention of
> reentry of a member function for all objects, the syntax is:
>
> struct MyClass {
> int MemberFunc(void) noreentry_all_objects(-1)
> {
> // The rest of the function goes here
> }
> };
>
> So you can only apply "noreentry" to a normal function, and you can
> only apply "noreentry_this_object" and "noreentry_all_objects" to a
> member function. Note that you cannot apply 'noreentry' to a member
> function because I don't want the confusion.
>
> Behind the scenes, here is how each of them would work. Firstly I've
> made a 'lock_guard' class to be used with 'atomic_flag':
>
> #include <atomic>
>
> class atomic_flag_guard {
> std::atomic_flag &flag;
> bool const previous;
> public:
> atomic_flag_guard(std::atomic_flag &arg) noexcept
> : flag(arg), previous(flag.test_and_set()) {}
>
> ~atomic_flag_guard(void) noexcept
> {
> if ( false == previous ) flag.clear();
> }
>
> bool failed(void) const noexcept { return previous; }
> };
>
> Here's the implementation of the normal function:
>
> int Func(void)
> {
> static std::atomic_flag flag;
> atomic_flag_guard myguard(flag);
> if ( myguard.failed() ) return -1;
> // The rest of the function goes here
> }
>
> Here's the implementation of the member function:
>
> struct MyClass {
> std::atomic_flag flag;
> int MemberFunc(void)
> {
> atomic_flag_guard myguard(flag);
> if ( myguard.failed() ) return -1;
> // The rest of the function goes here
> }
> };
>
> And here's the implementation of the member function whereby reentry
> is prevented for all objects:
>
> struct MyClass {
> int MemberFunc(void)
> {
> static std::atomic_flag flag;
> atomic_flag_guard myguard(flag);
> if ( myguard.failed() ) return -1;
> // The rest of the function goes here
> }
> };
>
> I had considered a few other little things, for example let's say you
> want the function to throw an exception rather than return -1. The
> syntax would be something like:
>
> int Func(void) noreentry( throw runtime_error("cannot re-enter") )
> {
> // The rest of the function goes here
> }
>
> This new feature could be kept simple, or tt could made be very
> complex with extra parameters to do stuff like:
> * Prevent reentry by other threads but not by the same thread
> * Prevent reentry by the same thread but not by other threads
> * Prevent reentry by all threads except the one with the specified thread::id
>
> Mostly this feature would be used in desktop GUI programming, although
> someone might find another use for it. It would even be useful purely
> as a debugging tool.

I think this idea is worth exploring whether or not it warrants a new
language feature.

Some missing pieces:
1. Some functions might be fine to execute concurrently, only reentering
from the same thread being problematic. Guarding free functions like
this could be done with a thread_local bool and a similar guard class
you have.

Not sure how individual objects and their member functions could be
guarded from same-thread reentry only. A thread_local set/map with
object addresses could probably work, but seems very intrusive.

2. Reentering for member functions is rarely about reentering the same
specific member function for a class. You often don't want to call
obj.foo() within a call to obj.bar() as well. You don't want to call
vec.insert() within vec.emplace_back(). It's about invariants. Some
non-const member functions break invariants within the body of the
function, only to restore them at the end of the function. You don't
want to call any of the member functions within the body of such a
function. I don't see a way to express this with your proposed syntax.

noreentry_this_object seems a bit intrusive in the sense that it
implicitly adds a non-static data member. It could probably be more
acceptable if the user could add the data member themselves, and they
had to specify it in the noreentry_this_object syntax.

All in all I think what you are trying to achieve might fit into the
"contracts" proposal, that is currently in progress, but I don't follow
it closely.

https://wg21.link/p2695

Cheers,
Lénárd