The change to osv::run() (and friends) to reuse the calling thread rather than start a new one to run a new shared-object introduced a new problem:
The new shared object may use pthread_key_create() to add destructors to run when closing threads. There is (currently) a global list of these destructors and once added they never get removed (even if the object is unloaded). The danger is that osv::run() runs code which adds such a destructors and allocates data (with pthread_setspecific()) and then the program is unloaded - but the thread will only exit (and the destructors run) later - and crash when trying to run the destructor pointing to already-deleted code. Previously we didn't have this problem, because osv::run() had a new thread, which exited (and ran the destructors) *before* the reference counter on the application dropped to 0 and the shared object was unloaded. A hacky workaround added in this patch is to run in application::start_and_join - used by osv::run, application::run_and_join and osv_execve - a new function which runs all these destructors. This assumes the thread starting these run functions did not have any thread- specific data (it would be deleted too!). It's a hack, but should be good enough for use cases like osv_execve() which anyway uses a fresh thread for running the application. Signed-off-by: Nadav Har'El <[email protected]> --- libc/pthread.hh | 4 ++++ core/app.cc | 9 +++++++++ libc/pthread.cc | 38 ++++++++++++++++++++++++++------------ 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/libc/pthread.hh b/libc/pthread.hh index 53e9db9..1a4d02c 100644 --- a/libc/pthread.hh +++ b/libc/pthread.hh @@ -20,6 +20,10 @@ extern "C" { #ifdef __cplusplus } + +namespace pthread_private { +void run_tsd_dtors(); +} #endif #endif /* PTHREAD_HH_ */ diff --git a/core/app.cc b/core/app.cc index 56fd18b..ad3145f 100644 --- a/core/app.cc +++ b/core/app.cc @@ -17,6 +17,7 @@ #include <algorithm> #include <boost/range/algorithm/transform.hpp> #include <osv/wait_record.hh> +#include "libc/pthread.hh" using namespace boost::range; @@ -251,6 +252,14 @@ void application::start_and_join(waiter* setup_waiter) } _thread = pthread_self(); // may be null if the caller is not a pthread. main(); + // FIXME: run_tsd_dtors() is a hack - If the new program registered a + // destructor via pthread_key_create() and this thread keeps living with + // data for this key, the destructor function, part of the new program, + // may be called after the program is unloaded - and crash. Let's run + // the destructors now. This is wrong if the calling program has its own + // thread-local data. It is fine if the thread was created specially for + // running start_and_join (or run_and_join() or osv::run()). + pthread_private::run_tsd_dtors(); sched::thread::current()->set_name(original_name); sched::thread::current()->set_app_runtime(original_app); original_app.reset(); diff --git a/libc/pthread.cc b/libc/pthread.cc index ac7b3f0..62db224 100644 --- a/libc/pthread.cc +++ b/libc/pthread.cc @@ -41,27 +41,41 @@ namespace pthread_private { __thread pthread_t current_pthread; __thread int cancel_state = PTHREAD_CANCEL_ENABLE; + // NOTE: currently, the list of keys and destructor for each is global, + // not per shared object or ELF namespace. So if a shared object uses + // pthread_key_create() but doesn't call pthread_key_delete() before + // exiting, the key will be leaked. This is relatively harmless (beyond + // running out of keys) unless the shared object is unloaded before the + // thread exits and the destructors are run. + // As a *hack* you can call run_tsd_dtors() before unloading the object, + // but this will run all dtors, not just those belonging to the unloaded + // object, so this is only useful on a thread especially created for + // running the object. __attribute__ ((init_priority ((int)init_prio::pthread))) mutex tsd_key_mutex; __attribute__ ((init_priority ((int)init_prio::pthread))) std::vector<bool> tsd_used_keys(tsd_nkeys); __attribute__ ((init_priority ((int)init_prio::pthread))) std::vector<void (*)(void*)> tsd_dtor(tsd_nkeys); + void run_tsd_dtors() { + bool done = false; + for (unsigned iter = 0; !done && iter < PTHREAD_DESTRUCTOR_ITERATIONS; ++iter) { + done = true; + for (unsigned i = 0; i < tsd_nkeys; ++i) { + if (tsd[i] && tsd_dtor[i]) { + void *val = tsd[i]; + tsd[i] = nullptr; + tsd_dtor[i](val); + done = false; + } + } + } + } + void __attribute__((constructor)) pthread_register_tsd_dtor_notifier() { sched::thread::register_exit_notifier([] { - bool done = false; - for (unsigned iter = 0; !done && iter < PTHREAD_DESTRUCTOR_ITERATIONS; ++iter) { - done = true; - for (unsigned i = 0; i < tsd_nkeys; ++i) { - if (tsd[i] && tsd_dtor[i]) { - void *val = tsd[i]; - tsd[i] = nullptr; - tsd_dtor[i](val); - done = false; - } - } - } + run_tsd_dtors(); }); } -- 2.7.4 -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
