It does help, the crash disappeared.
BR Justin
On 08/28/2016 02:31 PM, Nadav Har'El wrote:
Hi Justin, I hate this patch, but can you please check if it fixes
your crash-during-exit problem that you reported yesterday?
--
Nadav Har'El
[email protected] <mailto:[email protected]>
On Sun, Aug 28, 2016 at 3:29 PM, Nadav Har'El <[email protected]
<mailto:[email protected]>> wrote:
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]
<mailto:[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.
