You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by masaori335 <gi...@git.apache.org> on 2016/10/04 09:40:08 UTC
[GitHub] trafficserver pull request #1075: TS-4905: Set parent NULL after destroy() i...
GitHub user masaori335 opened a pull request:
https://github.com/apache/trafficserver/pull/1075
TS-4905: Set parent NULL after destroy() is called
[TS-4905](https://issues.apache.org/jira/browse/TS-4905)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/masaori335/trafficserver ts-4905
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1075.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 #1075
----
commit 9b6cd5d444bba0b5476ef67f98eea504dd6fa596
Author: Masaori Koshiba <ma...@apache.org>
Date: 2016-10-04T08:56:45Z
TS-4905: Set parent NULL after destroy() is called
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1075: TS-4905: Set parent NULL after destroy() i...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1075#discussion_r81987102
--- Diff: proxy/http/Http1ClientTransaction.cc ---
@@ -67,6 +67,7 @@ Http1ClientTransaction::transaction_done()
// If the parent session is not in the closed state, the destroy will not occur.
if (parent) {
parent->destroy();
+ parent = NULL;
--- End diff --
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.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1075: TS-4905: Set parent NULL after destroy() i...
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 closed the pull request at:
https://github.com/apache/trafficserver/pull/1075
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1075: TS-4905: Set parent NULL after destroy() is calle...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1075
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/924/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1075: TS-4905: Set parent NULL after destroy() i...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1075#discussion_r81791367
--- Diff: proxy/http/Http1ClientTransaction.cc ---
@@ -67,6 +67,7 @@ Http1ClientTransaction::transaction_done()
// If the parent session is not in the closed state, the destroy will not occur.
if (parent) {
parent->destroy();
+ parent = NULL;
--- End diff --
I don't think this is right, because ``destroy()`` might not destroy.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1075: TS-4905: Set parent NULL after destroy() i...
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1075#discussion_r81887418
--- Diff: proxy/http/Http1ClientTransaction.cc ---
@@ -67,6 +67,7 @@ Http1ClientTransaction::transaction_done()
// If the parent session is not in the closed state, the destroy will not occur.
if (parent) {
parent->destroy();
+ parent = NULL;
--- End diff --
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
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1075: TS-4905: Set parent NULL after destroy() is calle...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1075
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/818/ for details.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---