Re: release blockers?

On 16 October 2014 11:13, Torsten Förtsch <[email protected]> wrote:
> 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.

It looks like it was fixed by revision 594609 on the threading branch,
which I merged into httpd24threading and then into trunk.


>
> 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?
>

Yes, all of the work you did (some of it committed by gozer) on the
threading branch is now in trunk. In particular, revision 594601 is
the one that introduced ref-counted interpreter management for
threaded MPMs.