On Oct 3, 2014, at 8:35 AM, Sudheer Vinukonda <[email protected]> 
wrote:

> Hi James,
> 
> handleEvent() effectively calls the plugin (or in this case, SPDY layer)
> which may call TSFetchDestroy in error conditions. TSFetchDestroy sets
> contp to NULL, but, doesn¹t destroy FetchSM yet, since, it¹s in a tight
> loop protected by ³recursion² counter. When handleEvent returns, recursion
> is decremented and contp is already null, so, FetchSM gets destroyed.

Ah. In that case, there's a number of other places in InvokePluginExt where 
that can happen ...

> 
> Thanks,
> 
> Sudheer
> 
> On 10/3/14, 8:28 AM, "James Peach" <[email protected]> wrote:
> 
>> On Oct 3, 2014, at 6:29 AM, [email protected] wrote:
>> 
>>> Repository: trafficserver
>>> Updated Branches:
>>> refs/heads/master 33f651c90 -> d1b3dc66b
>>> 
>>> 
>>> [TS-3112] - Add null pointer check for contp to prevent core dump after
>>> handleEvent(TS_FETCH_EVENT_EXT_HEAD_DONE)
>>> 
>>> 
>>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>>> Commit: 
>>> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/d1b3dc66
>>> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/d1b3dc66
>>> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/d1b3dc66
>>> 
>>> Branch: refs/heads/master
>>> Commit: d1b3dc66b5725879949350890ab014cf235cae64
>>> Parents: 33f651c
>>> Author: Sudheer Vinukonda <[email protected]>
>>> Authored: Fri Oct 3 13:29:03 2014 +0000
>>> Committer: Sudheer Vinukonda <[email protected]>
>>> Committed: Fri Oct 3 13:29:03 2014 +0000
>>> 
>>> ----------------------------------------------------------------------
>>> proxy/FetchSM.cc | 3 +++
>>> 1 file changed, 3 insertions(+)
>>> ----------------------------------------------------------------------
>>> 
>>> 
>>> 
>>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d1b3dc66/proxy/
>>> FetchSM.cc
>>> ----------------------------------------------------------------------
>>> diff --git a/proxy/FetchSM.cc b/proxy/FetchSM.cc
>>> index d7b187a..4a79db4 100644
>>> --- a/proxy/FetchSM.cc
>>> +++ b/proxy/FetchSM.cc
>>> @@ -249,6 +249,9 @@ FetchSM::InvokePluginExt(int fetch_event)
>>>    has_sent_header = true;
>>>  }
>>> 
>>> +  if (!contp)
>>> +    goto out;
>>> +
>> 
>> There's a check for contp being NULL just 10 lines above here ... how can
>> it become NULL now?
>> 
>> 
>>>  if (!has_body()) {
>>>    contp->handleEvent(TS_FETCH_EVENT_EXT_BODY_DONE, this);
>>>    goto out;
>>> 
>> 
> 

Reply via email to