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

[Nils Carlson]

Yep. fixed. merged.

/Nils

On Wed, 23 Feb 2011, Yannick Brosseau wrote:

> 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
>>>
>>>
>
>