TS-4905: Set parent NULL after destroy() is called#1075
TS-4905: Set parent NULL after destroy() is called#1075masaori335 merged 1 commit intoapache:masterfrom
Conversation
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/924/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/818/ for details. |
| // If the parent session is not in the closed state, the destroy will not occur. | ||
| if (parent) { | ||
| parent->destroy(); | ||
| parent = NULL; |
There was a problem hiding this comment.
I don't think this is right, because destroy() might not destroy.
There was a problem hiding this comment.
Yeah, I'm not 100% sure this is right or not. But from the backtrace, destroy() calls free(). In this case we need to set parent NULL.
#0 Http1ClientSession::free (this=0x60ae0001cc80) at Http1ClientSession.cc:100
#1 0x00000000005c2386 in ProxyClientSession::handle_api_return (this=0x60ae0001cc80, event=60000) at ProxyClientSession.cc:206
#2 0x00000000005c1f25 in ProxyClientSession::do_api_callout (this=0x60ae0001cc80, id=TS_HTTP_SSN_CLOSE_HOOK) at ProxyClientSession.cc:177
#3 0x000000000065f787 in Http1ClientSession::destroy (this=0x60ae0001cc80) at Http1ClientSession.cc:93
#4 0x0000000000664d20 in Http1ClientTransaction::transaction_done (this=0x60ae0001cf60) at Http1ClientTransaction.cc:69
There was a problem hiding this comment.
Yes, destroy doesn't necessarily destroy. But from transaction_done(), the client transaction is done with the parent session. So even if the parent is still around, the client transaction should no longer be manipulating it.
The change looks good to me.
There was a problem hiding this comment.
OK, I don't like the fragility of how this is done, but I agree this change will fix the crash.
This cherry picks a review comment update from: apache#12290
TS-4905
IMO, the problem is calling
Http1ClientSession::get_netvc()afterHttp1ClientSession::free()is called.So set pointer to Http1ClientSession NULL after
Http1ClientSession::destroy()is called inHttp1ClientTransaction::transaction_done()