From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathieu.desnoyers@efficios.com (Mathieu Desnoyers) Date: Thu, 28 Apr 2011 10:57:22 -0400 Subject: [ltt-dev] [UST PATCH 2/3] ust-consumerd: fix exit race log corruption In-Reply-To: References: <1303935736-29208-1-git-send-email-jason.wessel@windriver.com> <1303935736-29208-2-git-send-email-jason.wessel@windriver.com> Message-ID: <20110428145722.GA24716@Krystal> * Nils Carlson (nils.carlson at ericsson.com) wrote: > merged. This is a step in the right direction, but we should not stop there: I recommend to: 1 - create a linked list of active threads. list_add should be called after each pthread_create. 2 - remove the call to pthread_detach(). 3 - before exit, iterate on each thread of the list, calling pthread_join for each of them. 4 - then we can remove the counter-based scheme proposed by this patch. Because with this scheme, we still race between exit() and thread memory free/teardown. It won't cause much problem in practice, but valgrind is likely to complain. Thanks, Mathieu > > On Wed, 27 Apr 2011, Jason Wessel wrote: > >> In the following scenario on an SMP system the ust-consumerd can end >> up not properly closing out file handles which leads to log >> corruption: >> * usttrace -m -l small_quick_app_lots_of_malloc_and_free >> * The app completes and usttrace sees and sends the SIGTERM to ust-consumerd >> * The ust-consumerd main thread will exit and the _exit() handlers >> kills off the remaining pthreads without everything getting closed out >> >> The solution to the problem is to introduce an active_thread count for >> the private ustconsumer_instance. This counter will be zeroed out >> when it is safe to completely shutdown the main thread, which will >> subsequently run the _exit() handlers. >> >> Signed-off-by: Jason Wessel >> --- >> include/ust/ustconsumer.h | 1 + >> libustconsumer/libustconsumer.c | 10 +++++++++- >> 2 files changed, 10 insertions(+), 1 deletions(-) >> >> diff --git a/include/ust/ustconsumer.h b/include/ust/ustconsumer.h >> index f99f8f1..cde8440 100644 >> --- a/include/ust/ustconsumer.h >> +++ b/include/ust/ustconsumer.h >> @@ -89,6 +89,7 @@ struct ustconsumer_instance { >> char *sock_path; >> pthread_mutex_t mutex; >> int active_buffers; >> + int active_threads; >> }; >> >> /** >> diff --git a/libustconsumer/libustconsumer.c b/libustconsumer/libustconsumer.c >> index 6f6d4bb..a723540 100644 >> --- a/libustconsumer/libustconsumer.c >> +++ b/libustconsumer/libustconsumer.c >> @@ -543,6 +543,10 @@ void *consumer_thread(void *arg) >> int result; >> sigset_t sigset; >> >> + pthread_mutex_lock(&args->instance->mutex); >> + args->instance->active_threads++; >> + pthread_mutex_unlock(&args->instance->mutex); >> + >> if(args->instance->callbacks->on_new_thread) >> args->instance->callbacks->on_new_thread(args->instance->callbacks); >> >> @@ -584,6 +588,10 @@ void *consumer_thread(void *arg) >> if(args->instance->callbacks->on_close_thread) >> args->instance->callbacks->on_close_thread(args->instance->callbacks); >> >> + pthread_mutex_lock(&args->instance->mutex); >> + args->instance->active_threads--; >> + pthread_mutex_unlock(&args->instance->mutex); >> + >> free((void *)args->channel); >> free(args); >> return NULL; >> @@ -735,7 +743,7 @@ int ustconsumer_start_instance(struct ustconsumer_instance *instance) >> >> if (instance->quit_program) { >> pthread_mutex_lock(&instance->mutex); >> - if(instance->active_buffers == 0) { >> + if(instance->active_buffers == 0 && instance->active_threads == 0) { >> pthread_mutex_unlock(&instance->mutex); >> break; >> } >> -- >> 1.7.1 >> >> >> _______________________________________________ >> ltt-dev mailing list >> ltt-dev at lists.casi.polymtl.ca >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev >> -- Mathieu Desnoyers Operating System Efficiency R&D Consultant EfficiOS Inc. http://www.efficios.com