From mboxrd@z Thu Jan  1 00:00:00 1970
From: dgoulet@efficios.com (David Goulet)
Date: Mon, 15 Oct 2012 13:37:05 -0400
Subject: [lttng-dev] [PATCH lttng-tools 1/4] Rename consumer threads and
 spawn them in daemon
In-Reply-To: <20121015172807.GA9034@Krystal>
References: <1350052235-12198-1-git-send-email-dgoulet@efficios.com>
	<20121013154113.GA29985@Krystal>
	<507C2E26.1030401@efficios.com> <20121015172807.GA9034@Krystal>
Message-ID: <507C49C1.3000801@efficios.com>



Mathieu Desnoyers:
>>>> diff --git a/src/common/consumer.c b/src/common/consumer.c
>>>> index 242b05b..055de1b 100644
>>>> --- a/src/common/consumer.c
>>>> +++ b/src/common/consumer.c
>>>> @@ -1131,6 +1131,8 @@ void lttng_consumer_destroy(struct lttng_consumer_local_data *ctx)
>>>>  		PERROR("close");
>>>>  	}
>>>>  	utils_close_pipe(ctx->consumer_splice_metadata_pipe);
>>>> +	/* This should trigger the metadata thread to exit */
>>>> +	close(ctx->consumer_metadata_pipe[1]);
>>>
>>> this is adding a close, but did not remove any other remove that might
>>> previously be in place elsewhere.
>>
>> So we got two possible error path which is either the poll thread fails
>> or the consumer could be destroy by hand even though the threads are
>> working well.
>>
>> Actually, this close should check if the value is valid and close it. To
>> be honest, this is just a shortcut since close(-1) does not fail and
>> ignoring the close error here since we are in the cleanup path anyway so
>> we don't necessarily care about the perror message.
>>
>> Anyhow, we have to handle both error path. An if plus set -1 after close
>> can be done so not to confuse.
> 
> if two threads can concurrently perform close on the same fd value, how
> can you prove there are no possible races ?

Nothing to prove, the race is possible. The point I was trying to
explain is that it does not matter actually since we are in a cleanup
code path.

Anyway, let's remove it since the data thread, when dying, will close
the metadata pipe anyway.

This will avoid more discussion for this small detail :).

David

> 
> Mathieu
>