On Wed, 8 Dec 2021 at 00:36, Jonathan Wakely wrote: > > On Tue, 7 Dec 2021 at 21:52, Florian Weimer wrote: > > > > * Jonathan Wakely: > > > > > On Tue, 7 Dec 2021, 21:20 Florian Weimer via Libstdc++, > > > <libstd...@gcc.gnu.org> > > > wrote: > > > > > > * Jonathan Wakely via Libstdc: > > > > > > > If necessary we could keep the terminate-on-cancellation behaviour as > > > > > > > _ZNSt18condition_variable4waitERSt11unique_lockISt5mutexE@GLIBCXX_3.4.11 > > > > and export the new behaviour as @@GLIBCXX_3.4.30, although this patch > > > > doesn't do that. > > > > > > Note that if this fix escapes into the wild and then you have to make > > > the symbol version change, you will break newer applications. In a few > > > cases in glibc, we proactively added aliases at a different symbol > > > version, but with the same implementation (at first). > > > > > > To be safe, we probably should preserve the old behaviour for the old > > > version of the symbol. If we decide that the new behaviour is always > > > preferable, we could change that later by making the old symbol an > > > alias for the new. If we don't decide that, we'll be glad we made it a > > > separate symbol. > > > > On the other hand, with separate versions, it's possible to reintroduce > > the old behavior at a later date, as a bugfix. It's not strictly > > necessary to do that work upfront. It's just nice to have this option. > > Ah yes, a new symbol version gives us more flexibility in every direction. > > > > I'll see if I can get it working with two versioned symbols. We don't > > > actually do that in libstdc++ currently, we only have a single version > > > of every symbol. > > > > Ping me if you want to discuss options. 8-> > > Thanks. I'll try it and let you know how I get on.
After resolving a PEBKAC issue, here's an incremental diff that preserves the old behaviour for the existing @GLIBCXX_3.4.11 symbol, but adds a new @@GLIBCXX_3.4.30 symbol that supports cancellation via __forced_unwind. Maybe we should also do this in the implementation of the old noexcept function: __attribute__((used)) void __nothrow_wait_cv::wait(std::unique_lock<std::mutex>& lock) noexcept { int old; int err = pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old); this->condition_variable::wait(lock); if (!err && old != PTHREAD_CANCEL_DISABLE) pthread_setcancelstate(old, &old); } This would prevent cancellation from terminating a process if it uses the old symbol. So we'd have a new symbol that supports cancellation, and an old one that safely disables it.
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/xml/manual/evolution.xml b/libstdc++-v3/doc/xml/manual/evolution.xml index 271d2225c3a..fd08cd84d20 100644 --- a/libstdc++-v3/doc/xml/manual/evolution.xml +++ b/libstdc++-v3/doc/xml/manual/evolution.xml @@ -1038,6 +1038,13 @@ The <literal>bitmap</literal>, <literal>mt</literal>, and <literal>pool</literal options for <option>--enable-libstdcxx-allocator</option> were removed. </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/src/c++11/compatibility-condvar.cc b/libstdc++-v3/src/c++11/compatibility-condvar.cc index 575d78055cb..439f1844e2c 100644 --- a/libstdc++-v3/src/c++11/compatibility-condvar.cc +++ b/libstdc++-v3/src/c++11/compatibility-condvar.cc @@ -54,4 +54,36 @@ _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," + "local" +); +#endif +#endif + #endif // _GLIBCXX_HAS_GTHREADS && _GLIBCXX_USE_C99_STDINT_TR1