Re: release blockers?

On 16/09/14 09:08, Jan Kaluža wrote:
>> Works for me last time I tried, but t/perl/ithreads* are not in the
>> release tarballs anyway, I think (because of persistent trouble with
>> them historically).
> 
> I've tried it with worker MPM and it fails for me, but I think the test
> is somehow broken. The comment in ithreads3.pm says:
> 
>     # now add an extra PerlCleanupHandler. It is run each time the
>     # interp is released. So it is run after Trans, MapToStorage and
>     # Fixup. In the response phase $counter should be 5. After Response
>     # it is run again but that is after.
> 
> But when checking the code for PerlCleanupHandler, it seems to be run
> once the request request_rec.pool is destroyed, which is in
> contradiction with the comment.
> 
> It then continues with:
> 
>     # This used to eat up all interpreters because
>     # modperl_interp_unselect
>     # calls modperl_config_request_cleanup that allocates a new interp
>     # to handle the cleanup.
> 
> This is also not a true in current code. modperl_interp_unselect does
> not call modperl_config_request_cleanup if I understand the current code.
> 
> I would say the test is outdated and I have no clue what was its goal.
> It clearly tries to count how many times are the handlers executed, but
> in its current state, it is not useful, because it tests the code paths
> we no longer have.

Back around 2.0.4 or so, we had several bug regarding interpreter
allocation/deallocation. I especially remember 2. The first one
allocated a separate interpreter for the response phase. So, you got
different interpreters for fixup and response for instance. Then we had
a complete mess with "PerlInterpScope handler". Perhaps the test is for
one of these cases.

We also had a bug that a PerlCleanupHandler was not called at all if
another perl handler hadn't been invoked prior in the request cycle. Not
sure if we have a test for that or if we even have fixed it.

I think all of these bugs should have been eliminated with the change
from simple flag-based interpreter management to refcount-based. I
believe this is what the current code is based upon, right?

Torsten