[GitHub] trafficserver pull request #1375: Incorrectly freeing Http1ClientSession set...

2017-01-26 Thread shinrich
Github user shinrich closed the pull request at:

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


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


[GitHub] trafficserver pull request #1375: Incorrectly freeing Http1ClientSession set...

2017-01-25 Thread shinrich
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 0x in ?? ()
#1 0x005e05ab in Http1ClientTransaction::do_io_write 
(this=0x2b512412e3d8, c=0x2b4ffd082470, nbytes=123, buf=0x2b50b4075e60, 
owner=false) at Http1ClientTransaction.h:45
#2 0x0063c7f5 in HttpTunnel::producer_run (this=0x2b4ffd082470, 
p=0x2b4ffd082670) at HttpTunnel.cc:916
#3 0x0063c103 in HttpTunnel::tunnel_run (this=0x2b4ffd082470, 
p_arg=0x2b4ffd082670) at HttpTunnel.cc:734
#4 0x005ff225 in HttpSM::setup_internal_transfer 
(this=0x2b4ffd080fd0, handler_arg=(int (HttpSM::)(HttpSM *, int, void *)) 
0x5f383c ) at HttpSM.cc:6222
#5 0x005ef6ae in HttpSM::handle_api_return (this=0x2b4ffd080fd0) at 
HttpSM.cc:1721
#6 0x005ef30b in HttpSM::state_api_callout (this=0x2b4ffd080fd0, 
event=6, data=0x0) at HttpSM.cc:1596
#7 0x005ee9e6 in HttpSM::state_api_callback (this=0x2b4ffd080fd0, 
event=6, data=0x0) at HttpSM.cc:1394
#8 0x00534ea1 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 
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.
---