You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by shinrich <gi...@git.apache.org> on 2016/07/02 13:29:16 UTC
[GitHub] trafficserver pull request #787: TS-4507: Fix SSN and TXN hook ordering.
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/787
TS-4507: Fix SSN and TXN hook ordering.
This change was motivated by dealing with ordering problems between SSN_CLOSE and TXN_CLOSE causing problems (crashes). This we addressed as follows
* The TXN_CLOSE is kicked off in the State Machine kill_this. This is the point were we know that the TXN really is going away. This is unchanged. What this patch adds is a call to transaction_done to let the ClientTransaction know that TXN_CLOSE has completed so the calculation about whether is it time to execute SSN_CLOSE or not. As such we cannot null out ua_session early.
* To support SSN_CLOSE accurately, we split the destroy logic into two parts: destroy and free. Destroy commits to deleting the session and it kicks off the SSN_CLOSE plugin. free performs the final resource reclamation of the session object.
In addition this PR includes the following fixes.
In ProxyClientSession::state_api_callout we schedule_in 10ms in the future if the plugin lock is not acquired. Saw ASAN use-after-free crashes when the Http2ClientSession is deleted but the schedule event remains and is triggered. Added a schedule_event member to track this case and cancel any outstanding schedule events on free.
Cleaned up handling regular events at the same time as plugin events. The original code relied on the subclasses overriding handle_api_event to handle the regular events, but the handler only handled the TIMEOUT event. Changed that to augment the subclasses' main event handler to call out to state_api_callout in the event of the plugin events.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shinrich/trafficserver ts-4507-2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/787.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 #787
----
commit ce7f22609b169b48e4883375ec42e78996827614
Author: Susan Hinrichs <sh...@ieee.org>
Date: 2016-07-02T13:10:11Z
TS-4507: Fix SSN and TXN hook ordering.
----
---
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 #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/787
Pushed new version of the branch that does not include the schedule_event and state_api_callout fixes. Filed TS-4663 and TS-4664 to track those issues. Will try to reproduce those fixes on new branches.
---
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 #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/787
---
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 #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/787
FreeBSD build *failed*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/432/ 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 issue #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/787
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/407/ 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 issue #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/787
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/325/ 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 issue #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/787
These fixes improved the stability of our 5.3.x + TS-3612 build. I'm heading out until July 14, but I wanted to make these changes available in case anyone else was seeing stability problems in this area.
---
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 #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/787
I've reviewed the changes and I don't think they can really be split up. Part of the reason is similar changes in both the HTTP/1 and HTTP/2 classes, which makes the change appear bigger than it is. I'm +1 on committing this, especially since we've been successfully testing them in production inside Yahoo!.
---
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 #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/trafficserver/pull/787
Is is possible to break the addditional fixes out into separate tickets and PRs?
---
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 #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/787
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/301/ 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 issue #787: TS-4507: Fix SSN and TXN hook ordering.
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/787
It would be possible to break out the schedule_event clean up and the state_api_callout into smaller patches. The rest of it really needs to be together. I need to move onto another project by Monday. I'll see what I can get done this afternoon. Otherwise these fixes will need to wait a while.
---
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.
---