On Jun 17, 2014, at 2:16 AM, Nick Kew <n...@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
>
