[lttng-dev] [PATCH lttng-tools 1/4] Rename consumer threads and spawn them in daemon

[mathieu.desnoyers@efficios.com (Mathieu Desnoyers)]

* David Goulet (dgoulet at efficios.com) wrote:
> 
> 
> 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.

what happens if we close FD 0, 1, 2 or another FD, due to this race ?

what happens if our code evolve to restart threads after errors, and we
leave this race in place, so it becomes hard to reproduce that in some
occasions we are closing random file descriptors ?

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

my point exactly :)

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

The devil is in the details, as someone famous said before me.

Thanks,

Mathieu

> 
> David
> 
> > 
> > Mathieu
> > 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com