[Bug libstdc++/98449] New: notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[Bug libstdc++/98449] New: notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[vini.ipsmaker at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

            Bug ID: 98449
           Summary: notify_all_at_thread_exit() should notify on cond
                    while lock is held to avoid a race
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vini.ipsmaker at gmail dot com
  Target Milestone: ---

Details of the issue can be found at: https://cplusplus.github.io/LWG/issue3343

I have a code base affected by this issue.

[Bug libstdc++/98449] notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-12-27
             Status|UNCONFIRMED                 |SUSPENDED

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The issue hasn't been accepted yet.

[Bug libstdc++/98449] [DR 3343] notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[vini.ipsmaker at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

Vinícius dos Santos Oliveira <vini.ipsmaker at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vini.ipsmaker at gmail dot com

--- Comment #2 from Vinícius dos Santos Oliveira <vini.ipsmaker at gmail dot com> ---
Created attachment 51126
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51126&action=edit
bugfix

Here's a patch fixing the issue.

If you want a codebase affected by the issue, here's the code for a thread
joining detached threads through a condition variable:
https://gitlab.com/emilua/emilua/-/blob/e5706426305733af02983ecb92a08dd8a00ebf2e/src/main.ypp#L275

Once the condition is met, both resources (mutex + condition variables) are
destroyed just before the application exits. Therefore I cannot use
notify_all_at_thread_exit (there would be a race there).

[Bug libstdc++/98449] [DR 3343] notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[vini.ipsmaker at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

--- Comment #3 from Vinícius dos Santos Oliveira <vini.ipsmaker at gmail dot com> ---
LLVM's libcxx already applied this 1-line patchset:
https://github.com/llvm/llvm-project/commit/64fc3cd55d586498dd21c5b3cfaa755793913772

Can we have the same here on GCC side (I've already submitted the patch
earlier).

[Bug libstdc++/98449] [DR 3343] notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[vini.ipsmaker at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

Vinícius dos Santos Oliveira <vini.ipsmaker at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|SUSPENDED                   |RESOLVED
         Resolution|---                         |FIXED

[Bug libstdc++/98449] [DR 3343] notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |SUSPENDED
         Resolution|FIXED                       |---

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Supending until the library defect report has been resolved.

[Bug libstdc++/98449] [DR 3343] notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 108861 has been marked as a duplicate of this bug. ***

[Bug libstdc++/98449] [DR 3343] notify_all_at_thread_exit() should notify on cond while lock is held to avoid a race

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98449

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Vinícius dos Santos Oliveira from comment #3)
> LLVM's libcxx already applied this 1-line patchset:
> https://github.com/llvm/llvm-project/commit/
> 64fc3cd55d586498dd21c5b3cfaa755793913772
> 
> Can we have the same here on GCC side (I've already submitted the patch
> earlier).

As you can obviously see from the libcxx commit, it's not a one-line change
because there should really be a test for it. Your patch is missing that.

I'll try to get the committee to make progress on the issue, but if you'd
waited for a reply before opening a pointless duplicate then I would have said
what Andrew said: we're waiting for the committee to approve the change before
changing our code.