From: Nadav Har'El <[email protected]>
Committer: Nadav Har'El <[email protected]>
Branch: master
osv::run(): run pthread TSD destructors before resetting app runtime
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]>
Message-Id: <[email protected]>
---
diff --git a/core/app.cc b/core/app.cc
--- 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
--- 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();
});
}
diff --git a/libc/pthread.hh b/libc/pthread.hh
--- 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_ */
--
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.
