[PATCH]Re: threaded server + scope=handler: modperl_response_handler vs. modperl_response_handler_cgi

[Torsten Foertsch <to...@gmx.net>]

On Monday 30 April 2007 12:08, Torsten Foertsch wrote:
> This means in both cases modperl_interp_select is called twice but for the
> perl-script case rcfg->interp = interp is set.
>
> modperl_interp_select first looks if rcfg->interp is set and returns it.
> Only if it's not it looks at other places for an interpreter.
>
> This means a perl-script handler needs 1 interpreter to handle the response
> phase while a modperl handler needs 2.

On Tuesday 01 May 2007 12:24, Torsten Foertsch wrote:
> On Monday 30 April 2007 22:22, Philippe M. Chiasson wrote:
> > And that doesn't seem logical at all. In all cases, since the initial
> > int= erpreter
> > is tied up in modperl_response_handler() a new one shouldn't be needed
> > to= dispatch
> > that handler part.
>
> I have found similar bug. Try
>
>   $r->push_handlers(PerlCleanupHandler=>'some_named_function')
>
> from a TranslationHandler or so.
[...]
> Or maybe a cleaner way is to use interp->refcnt. Simply increment it each
> time modperl_interp_select returns it. For what I can see refcnt isn't used
> at all by now. Yes, on a second thought that is what I'd prefer.


The attached patch fixes these 2 issues. It is against r534739. I have tested 
it with prefork and worker with a httpd 2.2.3 on linux.

What does it change?

1) it moves the handling of rcfg->interp from modperl_response_handler and 
modperl_response_handler_cgi to modperl_interp_select / 
modperl_interp_unselect.

2) If interp is saved to rcfg it counts as a reference to it. So refcnt is 
incremented.

All other changes are MP_TRACE macros to see what is going on.

Now the program flow is this:

If scope==request, connection or subrequest the PUTBACK flag is allways off. 
Interp is stored in the pool and modperl_interp_unselect is called by pool 
cleanup. Nothing has changed.

If scope==handler PUTBACK is on. Interp is not stored in a pool but allways in 
rcfg. So recursive calls to modperl_interp_select will find it there and 
increment its refcnt. Hence an equal number of calls to 
modperl_interp_unselect is required to really release the interp.

So, both modperl_response_handler allocate an interpreter then call 
modperl_callback_run_handlers which recursively allocates the same interp 
found in rcfg. The putback operation in modperl_callback_run_handlers then 
only decrements the refcnt and the putback in modperl_response_handler really 
does the unselect. But unselect has to call PerlCleanupHandlers. So 
modperl_interp_select is called again (while in modperl_interp_unselect), 
finds the interp in rcfg and increments its refcnt. After all cleanup 
handlers are run modperl_callback_run_handlers calls unselect which simply 
decrements the refcnt. Then modperl_config_request_cleanup returns to the 
outer modperl_interp_unselect and the interp is really freed.

Cleanup handlers installed via PerlCleanupHandler are thus called each time an 
interp is really freed.

$r->pool cleanups behave different. A pool cleanup_register also increments an 
interp's refcnt. Now if a pool cleanup is installed say in the translation 
phase the interp has allways a refcnt>0. So it is not released until the pool 
is destroyed. And since interp is stored in rcfg it is sticky to the request. 
All other phases after the pool cleanup installation will be handled by the 
same interp.

Torsten