You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Nick Kew <ni...@apache.org> on 2014/06/17 11:16:35 UTC
Lifetime of a Continuation
There's a common idiom in /plugins/ and /examples/.
Where a plugin uses a TSTransform, the TSTransform is created in the
event handler for the headers, and destroyed in the transform
continuation itself using:
if (TSVConnClosedGet(contp)) {
// ..... cleanup the ctx if applicable
TSContDestroy(contp);
return 0;
}
I adopted this idiom for Ironbee, which of course uses transforms for
both Request and Response data. This has now been fingered as the
culprit in a difficult crash, that happened only under load,
and affected only the input filter (so a plugin like gzip, esi,
or even null-transform that transforms output, wouldn't suffer).
Some debug output analysed exhaustively by my colleague and myself
pointed to TSContDataGet returning a pointer that appears valid but
cannot be dereferenced. The implication is, the transform function
has been called with bogus data, possibly after TSContDestroy.
Could this have been caused by "line noise" input?
Pursuing this line, we made a simple change to give
the transforms the lifetime of the txn itself.
1. Create the TSTransform at the start of the txn
in the handler for TS_EVENT_HTTP_TXN_START.
2. Destroy it at the end of the txn
in the handler for TS_EVENT_HTTP_TXN_CLOSE.
This simple lifetime change fixes the crash! I wonder if it would
be Good Practice to make an equivalent change in the bundled
transforms?
--
Nick Kew
Re: Lifetime of a Continuation
Posted by Nick Kew <ni...@apache.org>.
On Fri, 20 Jun 2014 21:55:19 +0100
Nick Kew <ni...@apache.org> wrote:
> Patch attached.
Bah. It seems the listserv stripped the patch.
Uploaded instead:
http://people.apache.org/~niq/patches/null-transform.patch
--
Nick Kew
Re: Lifetime of a Continuation
Posted by Nick Kew <ni...@apache.org>.
On 19 Jun 2014, at 18:54, Brian Geffon wrote:
> I think the one we were seeing was slightly different, nick would you mind
> sharing an example of what you changed with a simple transformation plugin?
OK, this is untested, but I've hacked null-transform to make what
looks like functionally the same as the change that fixed ironbee.
This is actually a slightly bigger change, because I had to introduce
things that didn't exist before, as opposed to just move them around!
The change is to give the TSVConn the lifetime of the tx.
Patch attached.
Re: Lifetime of a Continuation
Posted by Brian Geffon <br...@apache.org>.
I think the one we were seeing was slightly different, nick would you mind
sharing an example of what you changed with a simple transformation plugin?
On Thu, Jun 19, 2014 at 8:20 AM, Alan M. Carroll <
amc@network-geographics.com> wrote:
> Nick,
>
> You should talk to Brian G. He had a bug last month in transform that
> looked a lot like this. See TS-2820.
>
>
> https://issues.apache.org/jira/browse/TS-2820
>
> Tuesday, June 17, 2014, 11:16:35 AM, you wrote:
>
> > There's a common idiom in /plugins/ and /examples/.
>
> > Where a plugin uses a TSTransform, the TSTransform is created in the
> > event handler for the headers, and destroyed in the transform
> > continuation itself using:
>
> > if (TSVConnClosedGet(contp)) {
> > // ..... cleanup the ctx if applicable
> > TSContDestroy(contp);
> > return 0;
> > }
>
>
Re: Lifetime of a Continuation
Posted by "Alan M. Carroll" <am...@network-geographics.com>.
Nick,
You should talk to Brian G. He had a bug last month in transform that looked a lot like this. See TS-2820.
https://issues.apache.org/jira/browse/TS-2820
Tuesday, June 17, 2014, 11:16:35 AM, you wrote:
> There's a common idiom in /plugins/ and /examples/.
> Where a plugin uses a TSTransform, the TSTransform is created in the
> event handler for the headers, and destroyed in the transform
> continuation itself using:
> if (TSVConnClosedGet(contp)) {
> // ..... cleanup the ctx if applicable
> TSContDestroy(contp);
> return 0;
> }
Re: Lifetime of a Continuation
Posted by James Peach <jp...@apache.org>.
On Jun 17, 2014, at 2:16 AM, Nick Kew <ni...@apache.org> wrote:
> There's a common idiom in /plugins/ and /examples/.
>
> Where a plugin uses a TSTransform, the TSTransform is created in the
> event handler for the headers, and destroyed in the transform
> continuation itself using:
>
> if (TSVConnClosedGet(contp)) {
> // ..... cleanup the ctx if applicable
> TSContDestroy(contp);
> return 0;
> }
>
> I adopted this idiom for Ironbee, which of course uses transforms for
> both Request and Response data. This has now been fingered as the
> culprit in a difficult crash, that happened only under load,
> and affected only the input filter (so a plugin like gzip, esi,
> or even null-transform that transforms output, wouldn't suffer).
>
> Some debug output analysed exhaustively by my colleague and myself
> pointed to TSContDataGet returning a pointer that appears valid but
> cannot be dereferenced.
This pointer is not the stale contp? or the stale state pointer that you previously set on the continuation?
> The implication is, the transform function
> has been called with bogus data, possibly after TSContDestroy.
> Could this have been caused by "line noise" input?
>
> Pursuing this line, we made a simple change to give
> the transforms the lifetime of the txn itself.
> 1. Create the TSTransform at the start of the txn
> in the handler for TS_EVENT_HTTP_TXN_START.
> 2. Destroy it at the end of the txn
> in the handler for TS_EVENT_HTTP_TXN_CLOSE.
That doesn't look unreasonable, but I'd like to have a better understanding of the issue before committing to this pattern.
>
> This simple lifetime change fixes the crash! I wonder if it would
> be Good Practice to make an equivalent change in the bundled
> transforms?
>
> --
> Nick Kew
>