GitHub user shinrich opened a pull request:

    https://github.com/apache/trafficserver/pull/1375

    Incorrectly freeing Http1ClientSession setting up to return a error

    Saw a very deep stack.  The following is the stop of the stack.
    
    {code}
    #0 0x0000000000000000 in ?? ()
    #1 0x00000000005e05ab in Http1ClientTransaction::do_io_write 
(this=0x2b512412e3d8, c=0x2b4ffd082470, nbytes=123, buf=0x2b50b4075e60, 
owner=false) at Http1ClientTransaction.h:45
    #2 0x000000000063c7f5 in HttpTunnel::producer_run (this=0x2b4ffd082470, 
p=0x2b4ffd082670) at HttpTunnel.cc:916
    #3 0x000000000063c103 in HttpTunnel::tunnel_run (this=0x2b4ffd082470, 
p_arg=0x2b4ffd082670) at HttpTunnel.cc:734
    #4 0x00000000005ff225 in HttpSM::setup_internal_transfer 
(this=0x2b4ffd080fd0, handler_arg=(int (HttpSM::)(HttpSM *, int, void *)) 
0x5f383c <HttpSM::tunnel_handler(int, void)>) at HttpSM.cc:6222
    #5 0x00000000005ef6ae in HttpSM::handle_api_return (this=0x2b4ffd080fd0) at 
HttpSM.cc:1721
    #6 0x00000000005ef30b in HttpSM::state_api_callout (this=0x2b4ffd080fd0, 
event=60000, data=0x0) at HttpSM.cc:1596
    #7 0x00000000005ee9e6 in HttpSM::state_api_callback (this=0x2b4ffd080fd0, 
event=60000, data=0x0) at HttpSM.cc:1394
    #8 0x0000000000534ea1 in TSHttpTxnReenable (txnp=0x2b4ffd080fd0, 
event=TS_EVENT_HTTP_CONTINUE) at InkAPI.cc:5652
    {code}
    
    I spent some time grubbing through the core. It is interesting that the 
whole transaction is on the stack. The Http1ClientSession gets created in frame 
67. We crash because it has been deleted.
    
    I think the problem is in frame 4 in HttpSM::setup_internal_transfer. This 
is an error case. Specifically ATS is trying to return "HTTP/1.0 500 INKApi 
Error\r\nDate: Wed, 18 May 2016 20:21:46 GMT\r\nConnection: close\r\nServer: 
ATS/5.3.0\r\nContent-Length: 0\r\n\r\n" which gets set from 
HttpTransact::HandleApiErrorJump.
    
    But in setup_internal_transfer, we call tunnel.kill_tunnel to remove any 
previous static producers. But if there was a previous tunnel setup with our 
Http1ClientTransaction/Session as a consumer, it would call do_io_close on it 
which would likely free the Http1ClientSession object.
    
    Replaced the kill_tunnel with a reset which doesn't free the 
consumer/producer.  The crash went away.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/shinrich/trafficserver early-client-free

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/trafficserver/pull/1375.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1375
    
----
commit 95010edee4c5378b4f1f46174100e7a6e6115d13
Author: Susan Hinrichs <shinr...@ieee.org>
Date:   2017-01-25T20:46:56Z

    Incorrectly freeing Http1ClientSession while setting up to return a error 
header.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to