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....@googlemail.com> wrote:
On 15 May 2015 at 07:14, Jan Kaluža <jkal...@redhat.com> wrote:
On 05/14/2015 07:42 PM, Steve Hay wrote:
On 14 May 2015 at 12:48, Jan Kaluža <jkal...@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, <jkal...@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.
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-unsubscr...@perl.apache.org
For additional commands, e-mail: dev-h...@perl.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@perl.apache.org
For additional commands, e-mail: dev-h...@perl.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@perl.apache.org
For additional commands, e-mail: dev-h...@perl.apache.org
