You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by Leif Hedstrom <zw...@apache.org> on 2015/04/01 03:51:49 UTC

Re: trafficserver git commit: TS-3470 traffic_server --help segfaults when SPDY is enabled

> On Mar 31, 2015, at 1:39 PM, James Peach <jp...@apache.org> wrote:
> 
> 
>> On Mar 31, 2015, at 10:17 AM, zwoop@apache.org wrote:
>> 
>> Repository: trafficserver
>> Updated Branches:
>> refs/heads/master e13d8a6e8 -> e0de1d6ed
>> 
>> 
>> TS-3470 traffic_server --help segfaults when SPDY is enabled
>> 
>> 
> [snip]
>> diff --git a/lib/ts/ink_args.cc b/lib/ts/ink_args.cc
>> index 12ab5c0..91cebd8 100644
>> --- a/lib/ts/ink_args.cc
>> +++ b/lib/ts/ink_args.cc
>> @@ -323,5 +323,5 @@ usage(const ArgumentDescription *argument_descriptions, unsigned n_argument_desc
>>    }
>>    fprintf(stderr, " %s\n", argument_descriptions[i].description);
>>  }
>> -  exit(EX_USAGE);
>> +  _exit(EX_USAGE);
>> }
> 
> What's the underlying bug here? Note that using _exit() everywhere defeats tcmalloc leak detection and seems to be a bit of a cargo cult in our code ...



It’d segfault in SPDY as part of the destructor for some global object (I think). I didn’t feel it worthwhile to try to fix SPDY, since we’re killing it, and exiting out of the “usage()” like this hardly can have an impact on e.g. tcmalloc (would we care to test for memory leaks in “traffic_server —help”?).

We can certainly clean up the use of _exit(), I’m all for that.

— Leif


Re: trafficserver git commit: TS-3470 traffic_server --help segfaults when SPDY is enabled

Posted by James Peach <jp...@apache.org>.
> On Mar 31, 2015, at 6:51 PM, Leif Hedstrom <zw...@apache.org> wrote:
> 
>> 
>> On Mar 31, 2015, at 1:39 PM, James Peach <jp...@apache.org> wrote:
>> 
>> 
>>> On Mar 31, 2015, at 10:17 AM, zwoop@apache.org wrote:
>>> 
>>> Repository: trafficserver
>>> Updated Branches:
>>> refs/heads/master e13d8a6e8 -> e0de1d6ed
>>> 
>>> 
>>> TS-3470 traffic_server --help segfaults when SPDY is enabled
>>> 
>>> 
>> [snip]
>>> diff --git a/lib/ts/ink_args.cc b/lib/ts/ink_args.cc
>>> index 12ab5c0..91cebd8 100644
>>> --- a/lib/ts/ink_args.cc
>>> +++ b/lib/ts/ink_args.cc
>>> @@ -323,5 +323,5 @@ usage(const ArgumentDescription *argument_descriptions, unsigned n_argument_desc
>>>   }
>>>   fprintf(stderr, " %s\n", argument_descriptions[i].description);
>>> }
>>> -  exit(EX_USAGE);
>>> +  _exit(EX_USAGE);
>>> }
>> 
>> What's the underlying bug here? Note that using _exit() everywhere defeats tcmalloc leak detection and seems to be a bit of a cargo cult in our code ...
> 
> 
> 
> It’d segfault in SPDY as part of the destructor for some global object (I think). I didn’t feel it worthwhile to try to fix SPDY, since we’re killing it, and exiting out of the “usage()” like this hardly can have an impact on e.g. tcmalloc (would we care to test for memory leaks in “traffic_server —help”?).

FWIW, we have ~70 instances of _exit(). It's currently not possible to leak check with tcmalloc without hacking the build.

> 
> We can certainly clean up the use of _exit(), I’m all for that.

yes that would be good imho

J

Re: trafficserver git commit: TS-3470 traffic_server --help segfaults when SPDY is enabled

Posted by James Peach <jp...@apache.org>.
> On Mar 31, 2015, at 6:51 PM, Leif Hedstrom <zw...@apache.org> wrote:
> 
>> 
>> On Mar 31, 2015, at 1:39 PM, James Peach <jp...@apache.org> wrote:
>> 
>> 
>>> On Mar 31, 2015, at 10:17 AM, zwoop@apache.org wrote:
>>> 
>>> Repository: trafficserver
>>> Updated Branches:
>>> refs/heads/master e13d8a6e8 -> e0de1d6ed
>>> 
>>> 
>>> TS-3470 traffic_server --help segfaults when SPDY is enabled
>>> 
>>> 
>> [snip]
>>> diff --git a/lib/ts/ink_args.cc b/lib/ts/ink_args.cc
>>> index 12ab5c0..91cebd8 100644
>>> --- a/lib/ts/ink_args.cc
>>> +++ b/lib/ts/ink_args.cc
>>> @@ -323,5 +323,5 @@ usage(const ArgumentDescription *argument_descriptions, unsigned n_argument_desc
>>>   }
>>>   fprintf(stderr, " %s\n", argument_descriptions[i].description);
>>> }
>>> -  exit(EX_USAGE);
>>> +  _exit(EX_USAGE);
>>> }
>> 
>> What's the underlying bug here? Note that using _exit() everywhere defeats tcmalloc leak detection and seems to be a bit of a cargo cult in our code ...
> 
> 
> 
> It’d segfault in SPDY as part of the destructor for some global object (I think). I didn’t feel it worthwhile to try to fix SPDY, since we’re killing it, and exiting out of the “usage()” like this hardly can have an impact on e.g. tcmalloc (would we care to test for memory leaks in “traffic_server —help”?).

FWIW, we have ~70 instances of _exit(). It's currently not possible to leak check with tcmalloc without hacking the build.

> 
> We can certainly clean up the use of _exit(), I’m all for that.

yes that would be good imho

J