public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
From: "rachel at rachelmant dot com" <gcc-bugzilla@gcc.gnu.org> To: gcc-bugs@gcc.gnu.org Subject: [Bug libstdc++/110016] Possible miscodegen when inlining std::condition_variable::wait predicate causes deadlock Date: Mon, 29 May 2023 13:30:01 +0000 [thread overview] Message-ID: <bug-110016-4-oC3Ty2brAs@http.gcc.gnu.org/bugzilla/> (raw) In-Reply-To: <bug-110016-4@http.gcc.gnu.org/bugzilla/> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110016 Rachel Mant <rachel at rachelmant dot com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rachel at rachelmant dot com --- Comment #11 from Rachel Mant <rachel at rachelmant dot com> --- (In reply to Andrew Pinski from comment #9) Hi Andrew, original author of the ode here > So I think this is a bug in your code: > > Inside substrate::threadPool_t::finish, > we have: > > finished = true; > haveWork.notify_all(); > `finished` here should be a std::atomic in strict sequential ordering mode > If I change it to: > { > std::lock_guard<std::mutex> lock{workMutex}; > finished = true; > haveWork.notify_all(); > } > This will work, yes, except it will also cause mutex contention on the condition variable, and should not be necessary iff the atomic is working correctly as the order of operations between the threads should be: Thread 1 | Thread 2 ----------------------+------------------- finished = true | <in haveWork.wait()> haveWork.notify_all() | <wake-up> <joining begins> | lambda evlauation | <haveWork.wait() exists> The only way that the mutex can fix this is to add in the barrier that should already be there by virtue of the atomic being used std::memory_order_seq_cst unless there's also a TOCTOU issue in play. > Then I don't get a deadlock at all. > As I mentioned, I did think there was a race condition. > Here is what I think happened: > Thread26: thread 1 > checks finished, still false sets finished to be true > calls wait calls notify_all > ... notify_all happens > finally gets into futex_wait syscall .... > > And then thread26 never got the notification. > > With my change the check for finished has to wait till thread1 lets go of > the mutex (and the other way around). Do please correct us if we're wrong about our understanding on how the atomic should be working here. We're well aware that concurrency is /hard/ and getting things like this right is even harder. The thing that stands out, as Amy said, is that GCC is the *only* compiler on which we get this deadlock which is why she and we think this is a stdlib or codegen bug rather than our bug, but if you can say how it's not, then we're happy to accept that.
next prev parent reply other threads:[~2023-05-29 13:30 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-28 21:11 [Bug c++/110016] New: [12/13/14] " amy at amyspark dot me 2023-05-28 21:44 ` [Bug libstdc++/110016] " pinskia at gcc dot gnu.org 2023-05-29 0:47 ` [Bug libstdc++/110016] " amy at amyspark dot me 2023-05-29 1:03 ` pinskia at gcc dot gnu.org 2023-05-29 1:05 ` pinskia at gcc dot gnu.org 2023-05-29 1:05 ` pinskia at gcc dot gnu.org 2023-05-29 1:20 ` pinskia at gcc dot gnu.org 2023-05-29 1:27 ` pinskia at gcc dot gnu.org 2023-05-29 1:34 ` pinskia at gcc dot gnu.org 2023-05-29 1:47 ` pinskia at gcc dot gnu.org 2023-05-29 6:43 ` pinskia at gcc dot gnu.org 2023-05-29 13:30 ` rachel at rachelmant dot com [this message] 2023-05-29 14:28 ` pinskia at gcc dot gnu.org 2023-05-29 15:15 ` pinskia at gcc dot gnu.org 2023-05-29 15:18 ` rachel at rachelmant dot com 2023-05-29 15:37 ` pinskia at gcc dot gnu.org 2023-05-29 20:38 ` redi at gcc dot gnu.org
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bug-110016-4-oC3Ty2brAs@http.gcc.gnu.org/bugzilla/ \ --to=gcc-bugzilla@gcc.gnu.org \ --cc=gcc-bugs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).