Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. ModPerl>
  3. Dev
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c
steve.m.hay at googlemail
May 10, 2015, 5:47 AM
Post #1 of 16 (6122 views)
Permalink
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.
>
> Reasoning:
> 1. All calls of MP_INTERPa do not increment interp->refcnt, so refcnt
> used to be 0 before this commit. But there is always matching
> MP_INTERP_PUTBACK, which calls modperl_interp_unselect which decreases
> the refcnt, so it was possible to get negative refcnt or crash with
> threaded MPMs, because reference counting has been broken.
> 2. modperl_interp_select increases the refcount if it finds the PerlInterp in
> ccfg, so it makes sense to increase it (it means set to 1) during
> initialization too. Otherwise the refcnt would be incremented for the caller
> in some cases, but wouldn't be in other.
>
> This commit fixes the crash seen on worker MPM when PerlInterp has been used
> by two threads and the first one freed PerlInterp during
> modperl_interp_unselect.
>
> Modified:
> perl/modperl/trunk/src/modules/perl/modperl_interp.c
>

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.

Here's the output from "nmake test":

t\modules\proxy.t ....................... request has failed (the
response code was: 502)
see t/logs/error_log for more details
t\modules\proxy.t ....................... Dubious, test returned 9
(wstat 2304, 0x900)
Failed 1/1 subtests

And here's the verbose output:

t\modules\proxy.t .......................
# connecting to http://HPWIN7:8538/TestModules__proxy
1..1
# Running under perl version 5.020002 for MSWin32
# Current time local: Sun May 10 13:40:45 2015
# Current time GMT: Sun May 10 12:40:45 2015
# Using Test.pm version 1.26
# Using Apache/Test.pm version 1.39
request has failed (the response code was: 502)
see t/logs/error_log for more details
Dubious, test returned 9 (wstat 2304, 0x900)
Failed 1/1 subtests

Finally I have this in the error_log, which clearly shows a one minute
timeout from 13:40:49 to 13:41:50:

[Sun May 10 13:40:47.913364 2015] [example_hooks:notice] [pid
49036:tid 1404] x_create_connection()
[Sun May 10 13:40:47.914364 2015] [example_hooks:notice] [pid
49036:tid 1404] x_create_request()
[Sun May 10 13:40:47.914364 2015] [authz_core:debug] [pid 49036:tid
1404] mod_authz_core.c(834): [client 192.168.1.3:51023] AH01628:
authorization result: granted (no directives)
[Sun May 10 13:40:47.914364 2015] [proxy:debug] [pid 49036:tid 1404]
mod_proxy.c(1161): [client 192.168.1.3:51023] AH01143: Running scheme
http handler (attempt 0)
[Sun May 10 13:40:47.914364 2015] [proxy_fcgi:debug] [pid 49036:tid
1404] mod_proxy_fcgi.c(859): [client 192.168.1.3:51023] AH01076: url:
http://hpwin7:8538/TestModules__proxy_real proxyname: (null)
proxyport: 0
[Sun May 10 13:40:47.914364 2015] [proxy_fcgi:debug] [pid 49036:tid
1404] mod_proxy_fcgi.c(864): [client 192.168.1.3:51023] AH01077:
declining URL http://hpwin7:8538/TestModules__proxy_real
[Sun May 10 13:40:47.914364 2015] [proxy_scgi:debug] [pid 49036:tid
1404] mod_proxy_scgi.c(516): [client 192.168.1.3:51023] AH00865:
declining URL http://hpwin7:8538/TestModules__proxy_real
[Sun May 10 13:40:47.914364 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2138): AH00942: HTTP: has acquired connection for (*)
[Sun May 10 13:40:47.914364 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2192): [client 192.168.1.3:51023] AH00944: connecting
http://hpwin7:8538/TestModules__proxy_real to hpwin7:8538
[Sun May 10 13:40:47.915364 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2393): [client 192.168.1.3:51023] AH00947: connected
/TestModules__proxy_real to hpwin7:8538
[Sun May 10 13:40:48.915421 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2758): (OS 10061)No connection could be made because the
target machine actively refused it. : AH00957: HTTP: attempt to
connect to [fe80::cd76:804d:5c68:ef8c]:8538 (*) failed
[Sun May 10 13:40:49.916479 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2758): (OS 10061)No connection could be made because the
target machine actively refused it. : AH00957: HTTP: attempt to
connect to [fe80::183a:1874:3f57:fefc]:8538 (*) failed
[Sun May 10 13:40:49.917479 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2767): AH02824: HTTP: connection established with
192.168.1.3:8538 (*)
[Sun May 10 13:40:49.917479 2015] [example_hooks:notice] [pid
49036:tid 1404] x_create_connection()
[Sun May 10 13:40:49.917479 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2921): AH00962: HTTP: connection complete to
[fe80::cd76:804d:5c68:ef8c]:8538 (hpwin7)
[Sun May 10 13:40:49.919479 2015] [example_hooks:notice] [pid
49036:tid 1440] x_create_connection()
[Sun May 10 13:40:49.919479 2015] [example_hooks:notice] [pid
49036:tid 1440] x_create_request()
[Sun May 10 13:41:50.418939 2015] [proxy_http:error] [pid 49036:tid
1404] (OS 10060)A connection attempt failed because the connected
party did not properly respond after a period of time, or established
connection failed because connected host has failed to respond. :
[client 192.168.1.3:51023] AH01102: error reading status line from
remote server hpwin7:8538
[Sun May 10 13:41:50.418939 2015] [proxy_http:debug] [pid 49036:tid
1404] mod_proxy_http.c(1314): [client 192.168.1.3:51023] AH01103: read
timeout
[Sun May 10 13:41:50.418939 2015] [proxy_http:debug] [pid 49036:tid
1404] mod_proxy_http.c(1369): [client 192.168.1.3:51023] AH01105: NOT
Closing connection to client although reading from backend server
hpwin7:8538 failed.
[Sun May 10 13:41:50.418939 2015] [proxy:error] [pid 49036:tid 1404]
[client 192.168.1.3:51023] AH00898: Error reading from remote server
returned by http://HPWIN7:8538/TestModules__proxy_real
[Sun May 10 13:41:50.418939 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2153): AH00943: HTTP: has released connection for (*)
[Sun May 10 13:41:50.418939 2015] [proxy:debug] [pid 49036:tid 1404]
proxy_util.c(2863): [remote 192.168.1.3:8538] AH02642: proxy:
connection shutdown
[Sun May 10 13:41:50.418939 2015] [optional_fn_export:error] [pid
49036:tid 1404] AH01871: Optional function test said: GET
/TestModules__proxy HTTP/1.1
[Sun May 10 13:41:50.418939 2015] [optional_hook_import:error] [pid
49036:tid 1404] AH01866: Optional hook test said: GET
/TestModules__proxy HTTP/1.1
[Sun May 10 13:41:50.420939 2015] [authz_core:debug] [pid 49036:tid
1440] mod_authz_core.c(834): [client 192.168.1.3:51026] AH01628:
authorization result: granted (no directives)
[Sun May 10 13:41:50.420939 2015] [optional_fn_export:error] [pid
49036:tid 1440] AH01871: Optional function test said: GET
/TestModules__proxy_real HTTP/1.1
[Sun May 10 13:41:50.420939 2015] [optional_hook_import:error] [pid
49036:tid 1440] AH01866: Optional hook test said: GET
/TestModules__proxy_real HTTP/1.1

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
steve.m.hay at googlemail
May 10, 2015, 9:06 AM
Post #2 of 16 (6061 views)
Permalink
On 10 May 2015 at 13:47, Steve Hay <steve.m.hay@googlemail.com> 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.
>>
>> Reasoning:
>> 1. All calls of MP_INTERPa do not increment interp->refcnt, so refcnt
>> used to be 0 before this commit. But there is always matching
>> MP_INTERP_PUTBACK, which calls modperl_interp_unselect which decreases
>> the refcnt, so it was possible to get negative refcnt or crash with
>> threaded MPMs, because reference counting has been broken.
>> 2. modperl_interp_select increases the refcount if it finds the PerlInterp in
>> ccfg, so it makes sense to increase it (it means set to 1) during
>> initialization too. Otherwise the refcnt would be incremented for the caller
>> in some cases, but wouldn't be in other.
>>
>> This commit fixes the crash seen on worker MPM when PerlInterp has been used
>> by two threads and the first one freed PerlInterp during
>> modperl_interp_unselect.
>>
>> Modified:
>> perl/modperl/trunk/src/modules/perl/modperl_interp.c
>>
>
> 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.
>

That was testing with 2.4.12, but I have the same problem with 2.2.29
too, so it's not 2.4-specific.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
ntyni at debian
May 14, 2015, 2:24 AM
Post #3 of 16 (6047 views)
Permalink
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.

As a data point, this also seems to happen with 2.0.9-RC1 on Debian
kfreebsd-amd64.

https://buildd.debian.org/status/fetch.php?pkg=libapache2-mod-perl2&arch=kfreebsd-amd64&ver=2.0.9~rc1-1&stamp=1431587841

request has failed (the response code was: 502)
see t/logs/error_log for more details
t/modules/proxy.t .......................
# connecting to http://localhost:8538/TestModules__proxy
1..1
# Running under perl version 5.020002 for gnukfreebsd
# Current time local: Thu May 14 07:15:21 2015
# Current time GMT: Thu May 14 07:15:21 2015
# Using Test.pm version 1.26
# Using Apache/Test.pm version 1.39
Dubious, test returned 2 (wstat 512, 0x200)
Failed 1/1 subtests

I haven't been able to test yet whether it's reproducible there.
--
Niko Tyni ntyni@debian.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 14, 2015, 4:48 AM
Post #4 of 16 (6042 views)
Permalink
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?

Regards,
Jan Kaluza

> As a data point, this also seems to happen with 2.0.9-RC1 on Debian
> kfreebsd-amd64.
>
> https://buildd.debian.org/status/fetch.php?pkg=libapache2-mod-perl2&arch=kfreebsd-amd64&ver=2.0.9~rc1-1&stamp=1431587841
>
> request has failed (the response code was: 502)
> see t/logs/error_log for more details
> t/modules/proxy.t .......................
> # connecting to http://localhost:8538/TestModules__proxy
> 1..1
> # Running under perl version 5.020002 for gnukfreebsd
> # Current time local: Thu May 14 07:15:21 2015
> # Current time GMT: Thu May 14 07:15:21 2015
> # Using Test.pm version 1.26
> # Using Apache/Test.pm version 1.39
> Dubious, test returned 2 (wstat 512, 0x200)
> Failed 1/1 subtests
>
> I haven't been able to test yet whether it's reproducible there.
>

Attached Files:

Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
steve.m.hay at googlemail
May 14, 2015, 10:42 AM
Post #5 of 16 (6036 views)
Permalink
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?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 14, 2015, 11:14 PM
Post #6 of 16 (6038 views)
Permalink
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 .

Regards,
Jan Kaluza


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
steve.m.hay at googlemail
May 15, 2015, 12:56 AM
Post #7 of 16 (6037 views)
Permalink
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)?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
steve.m.hay at googlemail
May 15, 2015, 1:01 AM
Post #8 of 16 (6033 views)
Permalink
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.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 15, 2015, 2:57 AM
Post #9 of 16 (6030 views)
Permalink
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.

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
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 15, 2015, 3:26 AM
Post #10 of 16 (6035 views)
Permalink
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...

> 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
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 20, 2015, 2:49 AM
Post #11 of 16 (6008 views)
Permalink
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.

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
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 20, 2015, 6:51 AM
Post #12 of 16 (6011 views)
Permalink
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?

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
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
steve.m.hay at googlemail
May 24, 2015, 7:50 AM
Post #13 of 16 (5975 views)
Permalink
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
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 25, 2015, 2:11 AM
Post #14 of 16 (5969 views)
Permalink
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.

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
steve.m.hay at googlemail
May 27, 2015, 3:44 PM
Post #15 of 16 (5928 views)
Permalink
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
>>>
>
Re: svn commit: r1676417 - /perl/modperl/trunk/src/modules/perl/modperl_interp.c [ In reply to ]
jkaluza at redhat
May 28, 2015, 12:14 AM
Post #16 of 16 (5940 views)
Permalink
On 05/28/2015 12:44 AM, Steve Hay wrote:
>
> On 25 May 2015 10:11, "Jan Kaluža" <jkaluza@redhat.com
> <mailto: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
> <mailto: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
> <mailto:steve.m.hay@googlemail.com>> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 15 May 2015 at 07:14, Jan Kaluža <jkaluza@redhat.com
> <mailto: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
> <mailto: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
> <mailto: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.

OK, I should have time tomorrow for mod_perl, so I will commit my fix
for that threaded mpm freeze and removed PerlInterpScope.

Jan Kaluza

> 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
> <mailto:dev-unsubscribe@perl.apache.org>
> >>>>>> For additional commands, e-mail: dev-help@perl.apache.org
> <mailto:dev-help@perl.apache.org>
> >>>>>>
> >>>>>
> >>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> <mailto:dev-unsubscribe@perl.apache.org>
> >>>>> For additional commands, e-mail: dev-help@perl.apache.org
> <mailto:dev-help@perl.apache.org>
> >>>>>
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> <mailto:dev-unsubscribe@perl.apache.org>
> >>>> For additional commands, e-mail: dev-help@perl.apache.org
> <mailto:dev-help@perl.apache.org>
> >>>>
> >>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> <mailto:dev-unsubscribe@perl.apache.org>
> >>> For additional commands, e-mail: dev-help@perl.apache.org
> <mailto:dev-help@perl.apache.org>
> >>>
> >
>


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

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal