[jira] [Commented] (TS-3083) Double free of FetchSM

["Sudheer Vinukonda (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TS-3083?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14261868#comment-14261868 ] 

Sudheer Vinukonda commented on TS-3083:
---------------------------------------

Hi Leif - yes, this fix is just a "protection" against calling FetchSM's cleanUp() twice. 

FetchSM::InvokePluginExt() calls cleanUp(), if the continuation is null after handleEvent, which in itself, seems like a protection. In case of spdy, the continuation (SpdyClientSession) calls FetchSM::cleanUp() as well, when clearing a request. It's possible that the continuation could become null during handleEvent resulting in cleanUp() being called again via FetchSM::InvokePluginExt(). It's likely that the problems discovered in TS-3265 (incorrectly inserting null values in req_map possibly resulting in dangling FetchSM object) could explain (at least, some instances of) this issue.

Unfortunately, there's no other way of FetchSM knowing that cleanUp() had been already called by the continuation, other than this protection. Fwiw, in our production (v5.0), we saw this core rarely and we agreed internally on adding protection to just prevent the chance for calling cleanUp twice.

We could either revert the patch or change the logs to Error() to see if the issue is still happening after other recent fixes (mainly, TS-3265).




> Double free of FetchSM
> ----------------------
>
>                 Key: TS-3083
>                 URL: https://issues.apache.org/jira/browse/TS-3083
>             Project: Traffic Server
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 5.2.0
>            Reporter: bettydramit
>            Assignee: Sudheer Vinukonda
>            Priority: Blocker
>              Labels: crash
>             Fix For: 5.3.0
>
>         Attachments: fetch-clean.patch
>
>
> c++filt <a.txt 
> {code}
> /lib64/libpthread.so.0(+0xf710)[0x2b4c37949710]
> /usr/lib64/trafficserver/libtsutil.so.5(ink_atomiclist_pop+0x3e)[0x2b4c35abb64e]
> /usr/lib64/trafficserver/libtsutil.so.5(reclaimable_freelist_new+0x65)[0x2b4c35abc065]
> /usr/bin/traffic_server(MIOBuffer_tracker::operator()(long)+0x2b)[0x4a33db]
> /usr/bin/traffic_server(PluginVCCore::init()+0x2e3)[0x4d9903]
> /usr/bin/traffic_server(PluginVCCore::alloc()+0x11d)[0x4dcf4d]
> /usr/bin/traffic_server(TSHttpConnectWithPluginId+0x5d)[0x4b9e9d]
> /usr/bin/traffic_server(FetchSM::httpConnect()+0x74)[0x4a0224]
> /usr/bin/traffic_server(PluginVC::process_read_side(bool)+0x375)[0x4da675]
> /usr/bin/traffic_server(PluginVC::process_write_side(bool)+0x57a)[0x4dafca]
> /usr/bin/traffic_server(PluginVC::main_handler(int, void*)+0x315)[0x4dc9a5]
> /usr/bin/traffic_server(EThread::process_event(Event*, int)+0x8f)[0x73788f]
> /usr/bin/traffic_server(EThread::execute()+0x57b)[0x7381fb]
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)