[ltt-dev] [UST PATCH] libust: Fix multiple fd close during fork

[Yannick Brosseau]

On 2011-02-23 07:43, Nils Carlson wrote:
>
> This sort of fixes things in a wider sense as with the original patch
> we might leave trailing fds open and a non "auto" trace would have
> been neither destroyed nor have its fds closed...
>
> Hope its ok with you yannick. Keep up the good work!

that's fine with me. But shouldn't we also do this kind of loop for all
the "auto" in the ust_fork function?

>
> /Nils
>
> On Wed, 23 Feb 2011, Nils Carlson wrote:
>
>> Remove superfluous fd closes during fork and also destroy all
>> traces instead of just "auto".
>>
>> Reported-by: Yannick Brosseau <yannick.brosseau at gmail.com>
>> Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
>> ---
>> libust/tracectl.c |   17 ++++++++---------
>> 1 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/libust/tracectl.c b/libust/tracectl.c
>> index 1bd7229..1fc0118 100644
>> --- a/libust/tracectl.c
>> +++ b/libust/tracectl.c
>> @@ -1569,6 +1569,7 @@ static void ust_fork(void)
>> {
>>     struct ust_buffer *buf, *buf_tmp;
>>     struct ustcomm_sock *sock, *sock_tmp;
>> +    struct ust_trace *trace, *trace_tmp;
>>     int result;
>>
>>     /* FIXME: technically, the locks could have been taken before the
>> fork */
>> @@ -1589,18 +1590,16 @@ static void ust_fork(void)
>>     /* Delete all blocked consumers */
>>     cds_list_for_each_entry_safe(buf, buf_tmp, &open_buffers_list,
>>                  open_buffers_list) {
>> -        result = close(buf->data_ready_fd_read);
>> -        if (result == -1) {
>> -            PERROR("close");
>> -        }
>> -        result = close(buf->data_ready_fd_write);
>> -        if (result == -1) {
>> -            PERROR("close");
>> -        }
>>         cds_list_del(&buf->open_buffers_list);
>>     }
>>
>> -    ltt_trace_destroy("auto", 1);
>> +    /*
>> +     * FIXME: This could be prettier, we loop over the list twice and
>> +     * following good locking practice should lock around the loop
>> +     */
>> +    cds_list_for_each_entry_safe(trace, trace_tmp, &ltt_traces.head,
>> list) {
>> +        ltt_trace_destroy(trace->trace_name, 1);
>> +    }
>>
>>     /* Clean up the listener socket and epoll, keeping the scoket
>> file */
>>     ustcomm_del_named_sock(listen_sock, 1);
>> -- 
>> 1.7.1
>>
>>