On Wed, 01 Jan 2014 23:52:33 +0400, Serge Hallyn <[email protected]> wrote:
Quoting Andrey Mazo ([email protected]):
[snip]
diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c index 28691b0..36c4bd3 100644 --- a/src/lxc/lxclock.c +++ b/src/lxc/lxclock.c @@ -87,7 +87,7 @@ void unlock_mutex(pthread_mutex_t *l) int ret; if ((ret = pthread_mutex_unlock(l)) != 0) { - fprintf(stderr, "pthread_mutex_lock returned:%d %s", ret, strerror(ret)); + fprintf(stderr, "pthread_mutex_unlock returned:%d %s", ret, strerror(ret)); dump_stacktrace(); exit(1); } @@ -315,6 +315,21 @@ void process_unlock(void) unlock_mutex(&thread_mutex); } +/* One thread can do fork() while another one is holding a mutex. + * There is only one thread in child just after the fork(), so noone will ever release that mutex. + * We setup a "child" fork handler to unlock the mutex just after the fork(). + * For several mutex types, unlocking an unlocked mutex can lead to undefined behavior.Thanks for pointing this out. I have to say I don't like that we have to lock this... though it should be safe with our current usage (and I see Caglar has verified it :)
I don't like the implicit restriction on fork() inside process_lock() either, but, as far as I can see, it's the most reliable/portable approach (across mutex types).
An alternative would be to have a unlock_if_locked() for each lock, and do pthread_atfork(NULL, NULL, process_unlock_if_locked()). There would be no risk of a race since the child is single-threaded.
This is a good alternative. Another way is to trylock() the mutex before unlocking it. But I've decided not to go this way because of the following note for PTHREAD_MUTEX_DEFAULT in pthread_mutex_lock(3P): "Attempting to unlock the mutex if it was not locked by the calling thread results in undefined behavior."
But this is ok for now. Acked-by: Serge E. Hallyn <[email protected]> (Note I'm acking, but not applying until tomorrow)
Thank you!
+ * One way to deal with it is to setup "prepare" fork handler + * to lock the mutex before fork() and both "parent" and "child" fork handlers + * to unlock the mutex. + * This forbids doing fork() while explicitly holding the lock. + */ +__attribute__((constructor)) +static void process_lock_setup_atfork(void) +{ + pthread_atfork(process_lock, process_unlock, process_unlock); +} + /* Protects static const values inside the lxc_global_config_value funtion */ void static_lock(void) {
[snip] -- Andrey Mazo. _______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
