Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c

On 05/14/2015 07:42 PM, Steve Hay wrote:
> On 14 May 2015 at 12:48, Jan Kaluža <[email protected]> wrote:
>> On 05/14/2015 11:24 AM, Niko Tyni wrote:
>>>
>>> On Sun, May 10, 2015 at 01:47:19PM +0100, Steve Hay wrote:
>>>>
>>>> On 28 April 2015 at 07:51,  <[email protected]> wrote:
>>>>>
>>>>> Author: jkaluza
>>>>> Date: Tue Apr 28 06:51:12 2015
>>>>> New Revision: 1676417
>>>>>
>>>>> URL: http://svn.apache.org/r1676417
>>>>> Log:
>>>>> Initialize interp->refcnt to 1 in modperl_interp_select.
>>>
>>>
>>>> I cannot understand why, but since this patch was applied I find that
>>>> t\modules\proxy.t fails every time when I run the full "nmake test",
>>>> but it always succeeds when I run it in isolation so I'm at a loss to
>>>> find out what is going wrong. All other tests (apart from those known
>>>> Win32-specific failures documented in README) still pass. Reverting
>>>> the patch "fixes" the proxy.t problem, but probably isn't the right
>>>> solution.
>>
>>
>> It's caused by Perl_croak/modperl_croak.
>>
>> Lets take modperl_run_filter as an example. When following code-path is
>> executed ...
>>
>>                  modperl_croak(aTHX_ MODPERL_FILTER_ERROR,
>>                                "a filter calling $f->read "
>>                                "must return OK and not DECLINED");
>>
>> ... the MP_INTERP_PUTBACK is not reached for some reason (I presume it's
>> because of Perl_croak, but I don't understand why it stops the execution of
>> the rest of modperl_run_filter method).
>>
>> Because of that, the interp->refcnt is not decreased, and the interp is not
>> freed.
>>
>> I has been able to "fix" it by attached patch, but I would like to discuss
>> more generic way how to fix that problem...
>>
>> Any ideas?
>>
>
> modperl_croak() calls Perl_croak(), which is an XS interface to Perl's
> die() function, so surely you wouldn't expect anything immediately
> after it to be run?
>
> I'm not sure exactly where it does end up, though. It must be getting
> caught by some eval somewhere since we aren't exiting the process, but
> presumably it wouldn't be possible to do appropriate clean-up wherever
> it lands up unless there is some mechanism for registering required
> clean-up behaviour? Otherwise maybe we need to pass interp into
> modperl_croak(), or into a new version of that if not all cases
> require it, so that it can do the MP_INTERP_PUTBACK(interp, aTHX)
> call?
>

What worries me here a bit is that we would have to MP_INTEPR_PUTBACK 
the PerlInterp which is later used for PerlCroak, if I understand it right.

I have found out that usually when modperl_croak is called, the refcnt 
of the interp is above 1, so it wouldn't get freed prematurely, but still.

I think for now we should putback the interp only when interp->refcnt > 
1, it wouldn't fully fix all bugs, but lot of them would be fixed by that.

If someone knows how Perl_croak works and if it's possible to cleanup 
the interp after that, it would be great to share that info .

Regards,
Jan Kaluza