perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steve Hay <steve.m....@googlemail.com>
Subject Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c
Date Sun, 24 May 2015 14:50:02 GMT
On 20 May 2015 at 14:51, Jan Kaluža <jkaluza@redhat.com> wrote:
> On 05/20/2015 11:49 AM, Jan Kaluža wrote:
>>
>> On 05/15/2015 12:26 PM, Jan Kaluža wrote:
>>>
>>> On 05/15/2015 11:57 AM, Jan Kaluža wrote:
>>>>
>>>> On 05/15/2015 10:01 AM, Steve Hay wrote:
>>>>>
>>>>> On 15 May 2015 at 08:56, Steve Hay <steve.m.hay@googlemail.com>
wrote:
>>>>>>
>>>>>> On 15 May 2015 at 07:14, Jan Kaluža <jkaluza@redhat.com> wrote:
>>>>>>>
>>>>>>> On 05/14/2015 07:42 PM, Steve Hay wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 14 May 2015 at 12:48, Jan Kaluža <jkaluza@redhat.com>
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,  <jkaluza@apache.org>
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 .
>>>>>>>
>>>>>>
>>>>>> My understanding of Perl_croak() is that it either exits the process
>>>>>> (if not inside an eval()) or else calls the system's longjmp(), which
>>>>>> resumes execution from immediately after where the corresponding
>>>>>> setjmp() was called, having restored the process environment to the
>>>>>> original state at that point too.
>>>>>>
>>>>>> In the perl source, the setjmp()/longjmp() of eval()/die() are done
by
>>>>>> the JMPENV_PUSH in Perl_eval_sv() (maybe called from Perl_eval_pv())
>>>>>> and the JMPENV_JUMP in Perl_die_unwind(), called from Perl_vcroak().
>>>>>> The JMPENV* macros are in cop.h, and call
>>>>>> PerlProc_setjmp()/PerlProc_longjmp(), which are typically
>>>>>> setjmp()/longjmp(), or maybe sigsetjmp()/siglongjmp() if you have
>>>>>> them.
>>>>>>
>>>>>> I think you're right that we should probably check that interp->refcnt
>>>>>>>
>>>>>>> 1 if we go ahead and pass interp into modperl_croak(). There
aren't
>>>>>>
>>>>>> too many calls, so this may be workable; we also have a few call
>>>>>> MP_RUN_CROAK()/MP_RUN_CROAK_RESET() calls to look at too. What worries
>>>>>> me is the (much larger number of!) calls to Perl_croak(). They will
>>>>>> also not return, so we presumably need to do cleanup before each
one
>>>>>> of those too? Maybe we need a little wrapper function/macro to do
>>>>>> clean up and then call Perl_croak() and use that everywhere instead
of
>>>>>> Perl_croak() (including the call inside modperl_croak(), of course)?
>>>>>
>>>>>
>>>>> The other approach I mentioned earlier was to try to do the cleanup in
>>>>> the eval() where the die() has landed. If that's possible then it
>>>>> might be a cleaner approach.
>>>>>
>>>>> In this case, I think we're inside the eval_pv done in
>>>>> modperl_filter_resolve_init_handler(). I only see three other eval*()s
>>>>> (one more eval_pv() and two eval_sv()s) around the mod_perl C source
>>>>> code so this could be worth pursuing.
>>>>
>>>>
>>>> Hm, maybe stupid idea, but could we store the refcnt before the eval and
>>>> reset it after the eval? I presume that perlinterp is valid for the
>>>> evaluated code only when we are in the eval(), so if something increases
>>>> the refcnt in the eval() and forgets the decrease it later, we could
>>>> just decrease it ourselves using the set-eval-reset aproach.
>>>
>>>
>>> Not sure if we will be able to get modperl_interp_t * in these
>>> functions...
>>
>>
>> I found a way how to get modperl_interp_t *, but I don't think we will
>> be able to cleanup after eval_*/call_*. We don't know what the right
>> refcount should be. I will use the Perl_croak wrapper approach instead.
>
>
> OK, this works as expected, but there's different problem...
>
> The problem is that when PerlInterpScope is set to connection, the refcnt is
> increased because of connection->pool cleanup method
> modperl_interp_pool_cleanup.
>
> The refcnt is decreased once the connection->pool is cleaned-up, but if
> there are more requests being handled by the connection, the refcnt is
> connection->pool cleanup is not called after the first request and therefore
> the second request has to take another interpreter. This lead to deadlock
> later with more requests coming.
>
> What's the point of PerlInterpScope and how it should work ideally?

I'm sorry, I don't know the answer to this. I've never made use of
this directive myself, so all I know is what I can read in the help:

http://perl.apache.org/docs/2.0/user/config/config.html#C_PerlInterpScope_

I recall it coming up before regardng another refcnt-related patch.
There is some talk about it in the messages of this thread:

http://marc.info/?t=140191218700004&r=1&w=2



>
> Regards,
> Jan Kaluza
>
>
>> Jan Kaluza
>>
>>>> Otherwise we would need a way to find out that:
>>>>
>>>> a) Perl_croak has been executed - could be easy, I bet this is possible
>>>> somehow with Perl API.
>>>> b) There is actually lost reference - Not sure how to do that, we don't
>>>> know if PUTBACK has been called before the Perl_croak or not...
>>>>
>>>> Regards,
>>>> Jan Kaluza
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
>>>> For additional commands, e-mail: dev-help@perl.apache.org
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
>>> For additional commands, e-mail: dev-help@perl.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
>> For additional commands, e-mail: dev-help@perl.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> For additional commands, e-mail: dev-help@perl.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message