| Message ID | 20210616195218.1451153-1-rodgert@appliantology.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdc++: Fix for deadlock in std::counting_semaphore [PR100806] | expand |
On Wed, 16 Jun 2021 at 20:53, Thomas Rodgers <rodgert@appliantology.com> wrote: > > Same as previous version except removing the copyright notice from the > test. > > libstdc++-v3/ChangeLog: > libstdc++/PR100806 > * include/bits/semaphore_base.h (__atomic_semaphore::_M_release(): > Force _M_release() to wake all waiting threads. > * testsuite/30_threads/semaphore/100806.cc: New test. OK for trunk and 11, thanks. > --- > libstdc++-v3/include/bits/semaphore_base.h | 4 +- > .../testsuite/30_threads/semaphore/100806.cc | 60 +++++++++++++++++++ > 2 files changed, 63 insertions(+), 1 deletion(-) > create mode 100644 libstdc++-v3/testsuite/30_threads/semaphore/100806.cc > > diff --git a/libstdc++-v3/include/bits/semaphore_base.h b/libstdc++-v3/include/bits/semaphore_base.h > index 9a55978068f..c4565d7e560 100644 > --- a/libstdc++-v3/include/bits/semaphore_base.h > +++ b/libstdc++-v3/include/bits/semaphore_base.h > @@ -256,7 +256,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > if (__update > 1) > __atomic_notify_address_bare(&_M_counter, true); > else > - __atomic_notify_address_bare(&_M_counter, false); > + __atomic_notify_address_bare(&_M_counter, true); > +// FIXME - Figure out why this does not wake a waiting thread > +// __atomic_notify_address_bare(&_M_counter, false); > } > > private: > diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc b/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc > new file mode 100644 > index 00000000000..938c2793be1 > --- /dev/null > +++ b/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc > @@ -0,0 +1,60 @@ > +// { dg-options "-std=gnu++2a -pthread" } > +// { dg-do run { target c++2a } } > +// { dg-require-effective-target pthread } > +// { dg-require-gthreads "" } > +// { dg-add-options libatomic } > + > +#include <array> > +#include <iostream> > +#include <sstream> > + > +#include <thread> > +#include <semaphore> > +#include <mutex> > +#include <chrono> > +#include <vector> > + > +std::counting_semaphore<4> semaphore{6}; > + > +std::mutex mtx; > +std::vector<std::string> results; > + > +void thread_main(size_t x) > +{ > + semaphore.acquire(); > + std::this_thread::sleep_for(std::chrono::milliseconds(100)); > + semaphore.release(); > + { > + std::ostringstream stm; > + stm << "Thread " << x << " finished."; > + std::lock_guard g{ mtx }; > + results.push_back(stm.str()); > + } > +} > + > +int main() > +{ > + > + constexpr auto nthreads = 10; > + > + std::vector<std::thread> threads(nthreads); > + > + > + size_t counter{0}; > + for(auto& t : threads) > + { > + t = std::thread(thread_main, counter++); > + } > + > + for(auto& t : threads) > + { > + t.join(); > + { > + std::lock_guard g{ mtx }; > + for (auto&& r : results) > + std::cout << r << '\n'; > + std::cout.flush(); > + results.clear(); > + } > + } > +} > -- > 2.26.2 >
Tested x86_64-pc-linux-gnu. Committed to master, backported to releases/gcc-11. On Thu, Jun 17, 2021 at 9:46 AM Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > On Wed, 16 Jun 2021 at 20:53, Thomas Rodgers <rodgert@appliantology.com> > wrote: > > > > Same as previous version except removing the copyright notice from the > > test. > > > > libstdc++-v3/ChangeLog: > > libstdc++/PR100806 > > * include/bits/semaphore_base.h > (__atomic_semaphore::_M_release(): > > Force _M_release() to wake all waiting threads. > > * testsuite/30_threads/semaphore/100806.cc: New test. > > OK for trunk and 11, thanks. > > > > --- > > libstdc++-v3/include/bits/semaphore_base.h | 4 +- > > .../testsuite/30_threads/semaphore/100806.cc | 60 +++++++++++++++++++ > > 2 files changed, 63 insertions(+), 1 deletion(-) > > create mode 100644 libstdc++-v3/testsuite/30_threads/semaphore/100806.cc > > > > diff --git a/libstdc++-v3/include/bits/semaphore_base.h > b/libstdc++-v3/include/bits/semaphore_base.h > > index 9a55978068f..c4565d7e560 100644 > > --- a/libstdc++-v3/include/bits/semaphore_base.h > > +++ b/libstdc++-v3/include/bits/semaphore_base.h > > @@ -256,7 +256,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > if (__update > 1) > > __atomic_notify_address_bare(&_M_counter, true); > > else > > - __atomic_notify_address_bare(&_M_counter, false); > > + __atomic_notify_address_bare(&_M_counter, true); > > +// FIXME - Figure out why this does not wake a waiting thread > > +// __atomic_notify_address_bare(&_M_counter, false); > > } > > > > private: > > diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc > b/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc > > new file mode 100644 > > index 00000000000..938c2793be1 > > --- /dev/null > > +++ b/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc > > @@ -0,0 +1,60 @@ > > +// { dg-options "-std=gnu++2a -pthread" } > > +// { dg-do run { target c++2a } } > > +// { dg-require-effective-target pthread } > > +// { dg-require-gthreads "" } > > +// { dg-add-options libatomic } > > + > > +#include <array> > > +#include <iostream> > > +#include <sstream> > > + > > +#include <thread> > > +#include <semaphore> > > +#include <mutex> > > +#include <chrono> > > +#include <vector> > > + > > +std::counting_semaphore<4> semaphore{6}; > > + > > +std::mutex mtx; > > +std::vector<std::string> results; > > + > > +void thread_main(size_t x) > > +{ > > + semaphore.acquire(); > > + std::this_thread::sleep_for(std::chrono::milliseconds(100)); > > + semaphore.release(); > > + { > > + std::ostringstream stm; > > + stm << "Thread " << x << " finished."; > > + std::lock_guard g{ mtx }; > > + results.push_back(stm.str()); > > + } > > +} > > + > > +int main() > > +{ > > + > > + constexpr auto nthreads = 10; > > + > > + std::vector<std::thread> threads(nthreads); > > + > > + > > + size_t counter{0}; > > + for(auto& t : threads) > > + { > > + t = std::thread(thread_main, counter++); > > + } > > + > > + for(auto& t : threads) > > + { > > + t.join(); > > + { > > + std::lock_guard g{ mtx }; > > + for (auto&& r : results) > > + std::cout << r << '\n'; > > + std::cout.flush(); > > + results.clear(); > > + } > > + } > > +} > > -- > > 2.26.2 > > > >
diff --git a/libstdc++-v3/include/bits/semaphore_base.h b/libstdc++-v3/include/bits/semaphore_base.h index 9a55978068f..c4565d7e560 100644 --- a/libstdc++-v3/include/bits/semaphore_base.h +++ b/libstdc++-v3/include/bits/semaphore_base.h @@ -256,7 +256,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (__update > 1) __atomic_notify_address_bare(&_M_counter, true); else - __atomic_notify_address_bare(&_M_counter, false); + __atomic_notify_address_bare(&_M_counter, true); +// FIXME - Figure out why this does not wake a waiting thread +// __atomic_notify_address_bare(&_M_counter, false); } private: diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc b/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc new file mode 100644 index 00000000000..938c2793be1 --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/semaphore/100806.cc @@ -0,0 +1,60 @@ +// { dg-options "-std=gnu++2a -pthread" } +// { dg-do run { target c++2a } } +// { dg-require-effective-target pthread } +// { dg-require-gthreads "" } +// { dg-add-options libatomic } + +#include <array> +#include <iostream> +#include <sstream> + +#include <thread> +#include <semaphore> +#include <mutex> +#include <chrono> +#include <vector> + +std::counting_semaphore<4> semaphore{6}; + +std::mutex mtx; +std::vector<std::string> results; + +void thread_main(size_t x) +{ + semaphore.acquire(); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + semaphore.release(); + { + std::ostringstream stm; + stm << "Thread " << x << " finished."; + std::lock_guard g{ mtx }; + results.push_back(stm.str()); + } +} + +int main() +{ + + constexpr auto nthreads = 10; + + std::vector<std::thread> threads(nthreads); + + + size_t counter{0}; + for(auto& t : threads) + { + t = std::thread(thread_main, counter++); + } + + for(auto& t : threads) + { + t.join(); + { + std::lock_guard g{ mtx }; + for (auto&& r : results) + std::cout << r << '\n'; + std::cout.flush(); + results.clear(); + } + } +}