public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-5877] libstdc++: Allow std::condition_variable waits to be cancelled [PR103382]
@ 2021-12-09 23:27 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2021-12-09 23:27 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:9e18a25331fa25c3907249fede65a02c6817b06e

commit r12-5877-g9e18a25331fa25c3907249fede65a02c6817b06e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Dec 7 15:11:15 2021 +0000

    libstdc++: Allow std::condition_variable waits to be cancelled [PR103382]
    
    std::condition_variable::wait(unique_lock<mutex>&) is incorrectly marked
    noexcept, which means that the __forced_unwind exception used by NPTL
    cancellation will terminate the process. It should allow exceptions to
    pass through, so that a thread can be cleanly cancelled when waiting on
    a condition variable.
    
    The new behaviour is exported as a new version of the symbol, to avoid
    an ABI break for existing code linked to the non-throwing definition of
    the function. Code linked against older releases will have a reference
    to the @GLIBCXX_3.4.11 version, andcode compiled against the new
    libstdc++ will get a reference to the @@GLIBCXX_3.4.30 version.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/103382
            * config/abi/pre/gnu.ver (GLIBCXX_3.4.11): Do not export old
            symbol if .symver renaming is supported.
            (GLIBCXX_3.4.30): Export new symbol if .symver renaming is
            supported.
            * doc/xml/manual/evolution.xml: Document change.
            * doc/html/manual/api.html: Regenerate.
            * include/bits/std_mutex.h (__condvar::wait, __condvar::wait_until):
            Remove noexcept.
            * include/std/condition_variable (condition_variable::wait):
            Likewise.
            * src/c++11/condition_variable.cc (condition_variable::wait):
            Likewise.
            * src/c++11/compatibility-condvar.cc (__nothrow_wait_cv::wait):
            Define nothrow wrapper around std::condition_variable::wait and
            export the old symbol as an alias to it.
            * testsuite/30_threads/condition_variable/members/103382.cc: New test.

Diff:
---
 libstdc++-v3/config/abi/pre/gnu.ver                | 12 +++-
 libstdc++-v3/doc/html/manual/api.html              |  5 ++
 libstdc++-v3/doc/xml/manual/evolution.xml          |  7 +++
 libstdc++-v3/include/bits/std_mutex.h              |  6 +-
 libstdc++-v3/include/std/condition_variable        |  2 +-
 libstdc++-v3/src/c++11/compatibility-condvar.cc    | 31 ++++++++++
 libstdc++-v3/src/c++11/condition_variable.cc       |  2 +-
 .../condition_variable/members/103382.cc           | 66 ++++++++++++++++++++++
 8 files changed, 125 insertions(+), 6 deletions(-)

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 8f3c7b3827e..b747351a1b9 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1285,7 +1285,6 @@ GLIBCXX_3.4.11 {
     # condition_variable
     _ZNSt18condition_variable10notify_allEv;
     _ZNSt18condition_variable10notify_oneEv;
-    _ZNSt18condition_variable4waitERSt11unique_lockISt5mutexE;
     _ZNSt18condition_variableC1Ev;
     _ZNSt18condition_variableC2Ev;
     _ZNSt18condition_variableD1Ev;
@@ -1295,6 +1294,12 @@ GLIBCXX_3.4.11 {
     _ZNSt22condition_variable_anyD1Ev;
     _ZNSt22condition_variable_anyD2Ev;
 
+#ifndef HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT
+    # The original definition of this symbol gets versioned as @GLIBCXX_3.4.11
+    # if ".symver" is supported, or as @@GLIBCXX_3.4.11 otherwise.
+    _ZNSt18condition_variable4waitERSt11unique_lockISt5mutexE;
+#endif
+
     # thread
     _ZNSt6thread4joinEv;
     _ZNSt6thread6detachEv;
@@ -2401,6 +2406,11 @@ GLIBCXX_3.4.30 {
 
     _ZSt21__glibcxx_assert_fail*;
 
+#ifdef HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT
+    # The new definition of this symbol gets versioned as @@GLIBCXX_3.4.30
+    _ZNSt18condition_variable4waitERSt11unique_lockISt5mutexE;
+#endif
+
 } GLIBCXX_3.4.29;
 
 # Symbols in the support library (libsupc++) have their own tag.
diff --git a/libstdc++-v3/doc/html/manual/api.html b/libstdc++-v3/doc/html/manual/api.html
index 00701fa1044..1c7fdaa7394 100644
--- a/libstdc++-v3/doc/html/manual/api.html
+++ b/libstdc++-v3/doc/html/manual/api.html
@@ -450,4 +450,9 @@ options for <code class="option">--enable-libstdcxx-allocator</code> were remove
 For the <code class="literal">new</code> option, <code class="classname">std::allocator</code>
 no longer derives from <code class="classname">__gnu_cxx::new_allocator</code>;
 they both derive from <code class="classname">std::__new_allocator</code> instead.
+</p><p>
+<code class="function">std::condition_variable::wait</code> changed to be
+<code class="code">noexcept(false)</code> to allow thread cancellation exceptions to
+be thrown from <code class="function">pthread_cond_wait</code> without aborting
+the process.
 </p></div></div><div class="navfooter"><hr /><table width="100%" summary="Navigation footer"><tr><td width="40%" align="left"><a accesskey="p" href="abi.html">Prev</a> </td><td width="20%" align="center"><a accesskey="u" href="appendix_porting.html">Up</a></td><td width="40%" align="right"> <a accesskey="n" href="backwards.html">Next</a></td></tr><tr><td width="40%" align="left" valign="top">ABI Policy and Guidelines </td><td width="20%" align="center"><a accesskey="h" href="../index.html">Home</a></td><td width="40%" align="right" valign="top"> Backwards Compatibility</td></tr></table></div></body></html>
\ No newline at end of file
diff --git a/libstdc++-v3/doc/xml/manual/evolution.xml b/libstdc++-v3/doc/xml/manual/evolution.xml
index a169102ef43..34e44ee93e4 100644
--- a/libstdc++-v3/doc/xml/manual/evolution.xml
+++ b/libstdc++-v3/doc/xml/manual/evolution.xml
@@ -1041,6 +1041,13 @@ no longer derives from <classname>__gnu_cxx::new_allocator</classname>;
 they both derive from <classname>std::__new_allocator</classname> instead.
 </para>
 
+<para>
+<function>std::condition_variable::wait</function> changed to be
+<code>noexcept(false)</code> to allow thread cancellation exceptions to
+be thrown from <function>pthread_cond_wait</function> without aborting
+the process.
+</para>
+
 </section>
 
 </section>
diff --git a/libstdc++-v3/include/bits/std_mutex.h b/libstdc++-v3/include/bits/std_mutex.h
index 357c6891438..6618a54396f 100644
--- a/libstdc++-v3/include/bits/std_mutex.h
+++ b/libstdc++-v3/include/bits/std_mutex.h
@@ -149,7 +149,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     // Expects: Calling thread has locked __m.
     void
-    wait(mutex& __m) noexcept
+    wait(mutex& __m)
     {
       int __e __attribute__((__unused__))
 	= __gthread_cond_wait(&_M_cond, __m.native_handle());
@@ -157,14 +157,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     }
 
     void
-    wait_until(mutex& __m, timespec& __abs_time) noexcept
+    wait_until(mutex& __m, timespec& __abs_time)
     {
       __gthread_cond_timedwait(&_M_cond, __m.native_handle(), &__abs_time);
     }
 
 #ifdef _GLIBCXX_USE_PTHREAD_COND_CLOCKWAIT
     void
-    wait_until(mutex& __m, clockid_t __clock, timespec& __abs_time) noexcept
+    wait_until(mutex& __m, clockid_t __clock, timespec& __abs_time)
     {
       pthread_cond_clockwait(&_M_cond, __m.native_handle(), __clock,
 			     &__abs_time);
diff --git a/libstdc++-v3/include/std/condition_variable b/libstdc++-v3/include/std/condition_variable
index 4fcec6aa73d..3930cf487e9 100644
--- a/libstdc++-v3/include/std/condition_variable
+++ b/libstdc++-v3/include/std/condition_variable
@@ -92,7 +92,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     notify_all() noexcept;
 
     void
-    wait(unique_lock<mutex>& __lock) noexcept;
+    wait(unique_lock<mutex>& __lock);
 
     template<typename _Predicate>
       void
diff --git a/libstdc++-v3/src/c++11/compatibility-condvar.cc b/libstdc++-v3/src/c++11/compatibility-condvar.cc
index 575d78055cb..07c39d48f5a 100644
--- a/libstdc++-v3/src/c++11/compatibility-condvar.cc
+++ b/libstdc++-v3/src/c++11/compatibility-condvar.cc
@@ -54,4 +54,35 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
 
+#if ! _GLIBCXX_INLINE_VERSION
+// XXX GLIBCXX_ABI Deprecated
+// gcc-12.1
+// std::condition_variable::wait changed to noexcept(false)
+#if defined(_GLIBCXX_SYMVER_GNU) && defined(_GLIBCXX_SHARED) \
+    && defined(_GLIBCXX_HAVE_AS_SYMVER_DIRECTIVE) \
+    && defined(_GLIBCXX_HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT)
+namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
+{
+struct __nothrow_wait_cv : std::condition_variable
+{
+  void wait(std::unique_lock<std::mutex>&) noexcept;
+};
+
+__attribute__((used))
+void
+__nothrow_wait_cv::wait(std::unique_lock<std::mutex>& lock) noexcept
+{
+  this->condition_variable::wait(lock);
+}
+} // namespace __gnu_cxx
+
+// Export a noexcept wrapper around std::condition_variable::wait
+// with the original @GLIBCXX_3.4.11 symbol version.
+asm(
+  ".symver _ZN9__gnu_cxx17__nothrow_wait_cv4waitERSt11unique_lockISt5mutexE,"
+  "_ZNSt18condition_variable4waitERSt11unique_lockISt5mutexE@GLIBCXX_3.4.11"
+);
+#endif
+#endif
+
 #endif // _GLIBCXX_HAS_GTHREADS && _GLIBCXX_USE_C99_STDINT_TR1
diff --git a/libstdc++-v3/src/c++11/condition_variable.cc b/libstdc++-v3/src/c++11/condition_variable.cc
index ca7902e0373..2d7b8a19b9d 100644
--- a/libstdc++-v3/src/c++11/condition_variable.cc
+++ b/libstdc++-v3/src/c++11/condition_variable.cc
@@ -36,7 +36,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   condition_variable::~condition_variable() noexcept = default;
 
   void
-  condition_variable::wait(unique_lock<mutex>& __lock) noexcept
+  condition_variable::wait(unique_lock<mutex>& __lock)
   {
     _M_cond.wait(*__lock.mutex());
   }
diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable/members/103382.cc b/libstdc++-v3/testsuite/30_threads/condition_variable/members/103382.cc
new file mode 100644
index 00000000000..67396ebf323
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/103382.cc
@@ -0,0 +1,66 @@
+// { dg-options "-pthread" }
+// { dg-do run { target { *-*-linux* *-*-gnu* } } }
+// { dg-require-effective-target c++11 }
+// { dg-require-effective-target pthread }
+// { dg-require-gthreads "" }
+
+#include <condition_variable>
+#include <chrono>
+#include <mutex>
+#include <thread>
+
+// PR libstdc++/103382
+
+template<typename F>
+void
+test_cancel(F wait)
+{
+  std::mutex m;
+  std::condition_variable cv;
+  bool waiting = false;
+
+  std::thread t([&] {
+    std::unique_lock<std::mutex> lock(m);
+    waiting = true;
+    wait(cv, lock); // __forced_unwind exception should not terminate process.
+  });
+
+  // Ensure the condition variable is waiting before we cancel.
+  // This shouldn't be necessary because pthread_mutex_lock is not
+  // a cancellation point, but no harm in making sure we test what
+  // we intend to test: that cancel during a wait doesn't abort.
+  while (true)
+  {
+    std::unique_lock<std::mutex> lock(m);
+    if (waiting)
+      break;
+  }
+
+  pthread_cancel(t.native_handle());
+  t.join();
+}
+
+int main()
+{
+  test_cancel(
+      [](std::condition_variable& cv, std::unique_lock<std::mutex>& l) {
+	cv.wait(l);
+      });
+
+  test_cancel(
+      [](std::condition_variable& cv, std::unique_lock<std::mutex>& l) {
+	cv.wait(l, []{ return false; });
+      });
+
+  using mins = std::chrono::minutes;
+
+  test_cancel(
+      [](std::condition_variable& cv, std::unique_lock<std::mutex>& l) {
+	cv.wait_for(l, mins(1));
+      });
+
+  test_cancel(
+      [](std::condition_variable& cv, std::unique_lock<std::mutex>& l) {
+	cv.wait_until(l, std::chrono::system_clock::now() + mins(1));
+      });
+}


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-12-09 23:27 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 23:27 [gcc r12-5877] libstdc++: Allow std::condition_variable waits to be cancelled [PR103382] Jonathan Wakely

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).