[ltt-dev] [UST PATCH 2/3] ust-consumerd: fix exit race log corruption

[Mathieu Desnoyers]

* 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 <jason.wessel at windriver.com>
>> ---
>> 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