EricWF created this revision.
EricWF added reviewers: mclow.lists, danalbert, jroelofs.
EricWF added a subscriber: cfe-commits.
Hi,
When creating a new thread libc++ performs at least 2 allocations. The first
allocates a tuple of args and the functor that will be passed to the new
thread. The second allocation is for the thread local storage needed internally
by libc++. Currently the second allocation happens in the child thread, meaning
that if it throws the program will terminate with an uncaught bad alloc.
The solution to this is to allocate ALL memory in the parent thread and then
pass it to the child.
See https://llvm.org/bugs/show_bug.cgi?id=15638
http://reviews.llvm.org/D13748
Files:
include/thread
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
Index: test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
===================================================================
--- test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
+++ test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
@@ -19,23 +19,28 @@
#include <thread>
#include <new>
+#include <atomic>
#include <cstdlib>
#include <cassert>
#include "test_macros.h"
-unsigned throw_one = 0xFFFF;
+std::atomic<unsigned> throw_one(0xFFFF);
+std::atomic<unsigned> outstanding_new(0);
+
void* operator new(std::size_t s) throw(std::bad_alloc)
{
if (throw_one == 0)
throw std::bad_alloc();
--throw_one;
+ ++outstanding_new;
return std::malloc(s);
}
void operator delete(void* p) throw()
{
+ --outstanding_new;
std::free(p);
}
@@ -93,27 +98,58 @@
#endif
-int main()
-{
+// Test throwing std::bad_alloc
+//-----------------------------
+// Concerns:
+// A Each allocation performed during thread construction should be performed
+// in the parent thread so that std::terminate is not called if
+// std::bad_alloc is thrown by new.
+// B std::threads constructor should properly handle exceptions and not leak
+// memory.
+// Plan:
+// 1 Create a thread and count the number of allocations, 'N', it performs.
+// 2 For each allocation performed run a test where that allocation throws.
+// 2.1 check that the exception can be caught in the parent thread.
+// 2.2 Check that the functor has not been called.
+// 2.3 Check that no memory allocated by the creation of the thread is leaked.
+// 3 Finally check that a thread runs successfully if we throw after 'N+1'
+// allocations.
+void test_throwing_new_during_thread_creation() {
+ throw_one = 0xFFF;
{
std::thread t(f);
t.join();
- assert(f_run == true);
}
- f_run = false;
- {
- try
- {
- throw_one = 0;
+ const int numAllocs = 0xFFF - throw_one;
+ // i <= numAllocs means the last iteration is expected not to throw.
+ for (int i=0; i <= numAllocs; ++i) {
+ throw_one = i;
+ f_run = false;
+ unsigned old_outstanding = outstanding_new;
+ try {
std::thread t(f);
- assert(false);
- }
- catch (...)
- {
- throw_one = 0xFFFF;
- assert(!f_run);
+ assert(i == numAllocs); // Only final iteration will not throw.
+ t.join();
+ assert(f_run);
+ } catch (std::bad_alloc const&) {
+ assert(i < numAllocs);
+ assert(!f_run); // (2.2)
}
+ assert(old_outstanding == outstanding_new); // (2.3)
+ }
+ f_run = false;
+ throw_one = 0xFFF;
+}
+
+int main()
+{
+ test_throwing_new_during_thread_creation();
+ {
+ std::thread t(f);
+ t.join();
+ assert(f_run == true);
}
+
{
assert(G::n_alive == 0);
assert(!G::op_run);
Index: include/thread
===================================================================
--- include/thread
+++ include/thread
@@ -338,21 +338,21 @@
#ifndef _LIBCPP_HAS_NO_VARIADICS
-template <class _Fp, class ..._Args, size_t ..._Indices>
+template <class _TSp, class _Fp, class ..._Args, size_t ..._Indices>
inline _LIBCPP_INLINE_VISIBILITY
void
-__thread_execute(tuple<_Fp, _Args...>& __t, __tuple_indices<_Indices...>)
+__thread_execute(tuple<_TSp, _Fp, _Args...>& __t, __tuple_indices<_Indices...>)
{
- __invoke(_VSTD::move(_VSTD::get<0>(__t)), _VSTD::move(_VSTD::get<_Indices>(__t))...);
+ __invoke(_VSTD::move(_VSTD::get<1>(__t)), _VSTD::move(_VSTD::get<_Indices>(__t))...);
}
template <class _Fp>
-void*
-__thread_proxy(void* __vp)
+void* __thread_proxy(void* __vp)
{
- __thread_local_data().reset(new __thread_struct);
+ // _Fp = std::tuple< unique_ptr<__thread_struct>, Functor, Args...>
std::unique_ptr<_Fp> __p(static_cast<_Fp*>(__vp));
- typedef typename __make_tuple_indices<tuple_size<_Fp>::value, 1>::type _Index;
+ __thread_local_data().reset(_VSTD::get<0>(*__p).release());
+ typedef typename __make_tuple_indices<tuple_size<_Fp>::value, 2>::type _Index;
__thread_execute(*__p, _Index());
return nullptr;
}
@@ -362,9 +362,13 @@
>
thread::thread(_Fp&& __f, _Args&&... __args)
{
- typedef tuple<typename decay<_Fp>::type, typename decay<_Args>::type...> _Gp;
- _VSTD::unique_ptr<_Gp> __p(new _Gp(__decay_copy(_VSTD::forward<_Fp>(__f)),
- __decay_copy(_VSTD::forward<_Args>(__args))...));
+ typedef unique_ptr<__thread_struct> _TSPtr;
+ _TSPtr __tsp(new __thread_struct);
+ typedef tuple<_TSPtr, typename decay<_Fp>::type, typename decay<_Args>::type...> _Gp;
+ _VSTD::unique_ptr<_Gp> __p(
+ new _Gp(std::move(__tsp),
+ __decay_copy(_VSTD::forward<_Fp>(__f)),
+ __decay_copy(_VSTD::forward<_Args>(__args))...));
int __ec = pthread_create(&__t_, 0, &__thread_proxy<_Gp>, __p.get());
if (__ec == 0)
__p.release();
@@ -375,22 +379,34 @@
#else // _LIBCPP_HAS_NO_VARIADICS
template <class _Fp>
-void*
-__thread_proxy(void* __vp)
+struct __thread_invoke_pair {
+ // This type is used to pass memory for thread local storage and a functor
+ // to a newly created thread because std::pair doesn't work with
+ // std::unique_ptr in C++03.
+ __thread_invoke_pair(_Fp& __f) : __tsp_(new __thread_struct), __fn_(__f) {}
+ unique_ptr<__thread_struct> __tsp_;
+ _Fp __fn_;
+};
+
+template <class _Fp>
+void* __thread_proxy_cxx03(void* __vp)
{
- __thread_local_data().reset(new __thread_struct);
std::unique_ptr<_Fp> __p(static_cast<_Fp*>(__vp));
- (*__p)();
+ __thread_local_data().reset(__p->__tsp_.release());
+ (__p->__fn_)();
return nullptr;
}
template <class _Fp>
thread::thread(_Fp __f)
{
- std::unique_ptr<_Fp> __p(new _Fp(__f));
- int __ec = pthread_create(&__t_, 0, &__thread_proxy<_Fp>, __p.get());
+
+ typedef __thread_invoke_pair<_Fp> _InvokePair;
+ typedef std::unique_ptr<_InvokePair> _PairPtr;
+ _PairPtr __pp(new _InvokePair(__f));
+ int __ec = pthread_create(&__t_, 0, &__thread_proxy_cxx03<_InvokePair>, __pp.get());
if (__ec == 0)
- __p.release();
+ __pp.release();
else
__throw_system_error(__ec, "thread constructor failed");
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
