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 Wed, 27 May 2015 22:44:07 GMT
On 25 May 2015 10:11, "Jan Kaluža" <jkaluza@redhat.com> wrote:
>
> On 05/24/2015 04:50 PM, Steve Hay wrote:
>>
>> 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_
>
>
> Could we drop PerlInterpScope? It does not work as expected and I think
it cannot be fixed easily.
>
> In case of "PerlInterPScope connection", you can simply deadlock the
mod_perl when sending more small requests using the same connection,
because every request takes another interpreter and interpreter are not put
back untill the connection is dispatched.
>
> There is no easy way how to solve that.
>
> I think the original idea behind PerlInterpScope has been to make the
performance better when the single interpreter has been allocated and
unallocated often during single connection, but with the pipelining, this
is not easy to achieve with current mod_perl code.
>
> I would vote for removing the PerlInterpScope when building against
httpd-2.4.x at least.

If we can't easily fix it then I would certainly rather drop it (for now at
least) than hold up 2.0.9 any longer.

We can put a note in the Changes file about it to alert anyone that's
currently using it, and see what feedback we get before investing our
limited resources into trying to fix it.

Steve

>
> Regards,
> Jan Kaluza
>
>
>> 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
>>>
>

Mime
View raw message