diff --git a/repos/libports/src/lib/pthread/thread.cc b/repos/libports/src/lib/pthread/thread.cc index 88a93c54b..dda602b6c 100644 --- a/repos/libports/src/lib/pthread/thread.cc +++ b/repos/libports/src/lib/pthread/thread.cc @@ -13,6 +13,7 @@ */ #include +#include #include #include #include @@ -26,24 +27,6 @@ using namespace Genode; -/* - * Structure to handle self-destructing pthreads. - */ -struct thread_cleanup : List::Element -{ - pthread_t thread; - - thread_cleanup(pthread_t t) : thread(t) { } - - ~thread_cleanup() { - if (thread) - delete thread; - } -}; - -static Lock pthread_cleanup_list_lock; -static List pthread_cleanup_list; - void * operator new(__SIZE_TYPE__ size) { return malloc(size); } void operator delete (void * p) { return free(p); } @@ -72,10 +55,43 @@ void pthread::Thread_object::entry() _stack_addr = (void *)info.base; _stack_size = info.top - info.base; - void *exit_status = _start_routine(_arg); + pthread_exit(_start_routine(_arg)); +} + + +void pthread::join(void **retval) +{ + struct Check : Libc::Suspend_functor + { + bool retry { false }; + + pthread &_thread; + + Check(pthread &thread) : _thread(thread) { } + + bool suspend() override + { + retry = !_thread._exiting; + return retry; + } + } check(*this); + + do { + Libc::suspend(check); + } while (check.retry); + + _join_lock.lock(); + + if (retval) + *retval = _retval; +} + + +void pthread::cancel() +{ _exiting = true; Libc::resume_all(); - pthread_exit(exit_status); + _join_lock.unlock(); } @@ -136,20 +152,9 @@ extern "C" { int pthread_join(pthread_t thread, void **retval) { - struct Check : Libc::Suspend_functor - { - bool suspend() override { - return true; - } - } check; + thread->join(retval); - while (!thread->exiting()) { - Libc::suspend(check); - } - - - thread->join(); - *((int **)retval) = 0; + delete thread; return 0; } @@ -178,38 +183,17 @@ extern "C" { } - void pthread_cleanup() - { - { - Lock_guard lock_guard(pthread_cleanup_list_lock); - - while (thread_cleanup * t = pthread_cleanup_list.first()) { - pthread_cleanup_list.remove(t); - delete t; - } - } - } - int pthread_cancel(pthread_t thread) { - /* cleanup threads which tried to self-destruct */ - pthread_cleanup(); - - if (pthread_equal(pthread_self(), thread)) { - Lock_guard lock_guard(pthread_cleanup_list_lock); - pthread_cleanup_list.insert(new thread_cleanup(thread)); - } else - delete thread; - + thread->cancel(); return 0; } + void pthread_exit(void *value_ptr) { - pthread_cancel(pthread_self()); - - Lock lock; - while (true) lock.lock(); + pthread_self()->exit(value_ptr); + Genode::sleep_forever(); } diff --git a/repos/libports/src/lib/pthread/thread.h b/repos/libports/src/lib/pthread/thread.h index b65636560..c8728b040 100644 --- a/repos/libports/src/lib/pthread/thread.h +++ b/repos/libports/src/lib/pthread/thread.h @@ -60,8 +60,6 @@ extern "C" { * defined as 'struct pthread*' in '_pthreadtypes.h' */ struct pthread; - - void pthread_cleanup(); } @@ -104,7 +102,6 @@ struct pthread : Genode::Noncopyable, Genode::Thread::Tls::Base { start_routine_t _start_routine; void *_arg; - bool _exiting = false; void *&_stack_addr; size_t &_stack_size; @@ -149,6 +146,23 @@ struct pthread : Genode::Noncopyable, Genode::Thread::Tls::Base pthread_registry().insert(this); } + bool _exiting = false; + + /* + * The join lock is needed because 'Libc::resume_all()' uses a + * 'Signal_transmitter' which holds a reference to a signal context + * capability, which needs to be released before the thread can be + * destroyed. + * + * Also, we cannot use 'Genode::Thread::join()', because it only + * returns when the thread entry function returns, which does not + * happen with 'pthread_cancel()'. + */ + Genode::Lock _join_lock { Genode::Lock::LOCKED }; + + /* return value for 'pthread_join()' */ + void *_retval = PTHREAD_CANCELED; + /* attributes for 'pthread_attr_get_np()' */ void *_stack_addr = nullptr; size_t _stack_size = 0; @@ -191,12 +205,20 @@ struct pthread : Genode::Noncopyable, Genode::Thread::Tls::Base } void start() { _thread.start(); } - bool exiting() const - { - return _thread_object->_exiting; - } - void join() { _thread.join(); } + void join(void **retval); + + /* + * Inform the thread calling 'pthread_join()' that this thread can be + * destroyed. + */ + void cancel(); + + void exit(void *retval) + { + _retval = retval; + cancel(); + } void *stack_addr() const { return _stack_addr; } size_t stack_size() const { return _stack_size; } diff --git a/repos/libports/src/lib/pthread/thread_create.cc b/repos/libports/src/lib/pthread/thread_create.cc index d94316021..a1042e479 100644 --- a/repos/libports/src/lib/pthread/thread_create.cc +++ b/repos/libports/src/lib/pthread/thread_create.cc @@ -25,9 +25,6 @@ extern "C" int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg) { - /* cleanup threads which tried to self-destruct */ - pthread_cleanup(); - size_t const stack_size = (attr && *attr && (*attr)->stack_size) ? (*attr)->stack_size : Libc::Component::stack_size(); diff --git a/repos/libports/src/test/pthread/main.cc b/repos/libports/src/test/pthread/main.cc index c27c5348b..428420451 100644 --- a/repos/libports/src/test/pthread/main.cc +++ b/repos/libports/src/test/pthread/main.cc @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -52,7 +53,44 @@ void *thread_func(void *arg) return 0; } -void *thread_func_self_destruct(void *arg) { return 0; } +/* + * Test self-destructing threads with 'pthread_join()', both when created and + * joined by the main thread and when created and joined by a pthread. + */ + +void test_self_destruct(void *(*start_routine)(void*), uintptr_t num_iterations) +{ + for (uintptr_t i = 0; i < num_iterations; i++) { + + pthread_t t; + void *retval; + + if (pthread_create(&t, 0, start_routine, (void*)i) != 0) { + printf("error: pthread_create() failed\n"); + exit(-1); + } + + pthread_join(t, &retval); + + if (retval != (void*)i) { + printf("error: return value does not match\n"); + exit(-1); + } + } +} + +void *thread_func_self_destruct2(void *arg) +{ + return arg; +} + +void *thread_func_self_destruct(void *arg) +{ + test_self_destruct(thread_func_self_destruct2, 2); + + return arg; +} + static inline void compare_semaphore_values(int reported_value, int expected_value) { @@ -120,13 +158,25 @@ int main(int argc, char **argv) for (int i = 0; i < NUM_THREADS; i++) if (thread[i].thread_args.thread_id_self != thread[i].thread_id_create) { printf("error: thread IDs don't match\n"); + return -1; } printf("main thread: destroying the threads\n"); - for (int i = 0; i < NUM_THREADS; i++) + for (int i = 0; i < NUM_THREADS; i++) { + + void *retval; + pthread_cancel(thread[i].thread_id_create); + pthread_join(thread[i].thread_id_create, &retval); + + if (retval != PTHREAD_CANCELED) { + printf("error: return value is not PTHREAD_CANCELED\n"); + return -1; + } + } + printf("main thread: destroying the semaphores\n"); for (int i = 0; i < NUM_THREADS; i++) @@ -134,13 +184,7 @@ int main(int argc, char **argv) printf("main thread: create pthreads which self de-struct\n"); - for (unsigned i = 0 ; i < 100; i++) { - pthread_t t; - if (pthread_create(&t, 0, thread_func_self_destruct, 0) != 0) { - printf("error: pthread_create() failed\n"); - return -1; - } - } + test_self_destruct(thread_func_self_destruct, 100); printf("--- returning from main ---\n"); return 0; diff --git a/repos/ports/src/virtualbox/thread.cc b/repos/ports/src/virtualbox/thread.cc index f78dbf15d..446099199 100644 --- a/repos/ports/src/virtualbox/thread.cc +++ b/repos/ports/src/virtualbox/thread.cc @@ -116,9 +116,6 @@ static int create_thread(pthread_t *thread, const pthread_attr_t *attr, extern "C" int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine) (void *), void *arg) { - /* cleanup threads which tried to self-destruct */ - pthread_cleanup(); - PRTTHREADINT rtthread = reinterpret_cast(arg); /* retry thread creation once after CPU session upgrade */