You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Randy Kobes <ra...@theoryx5.uwinnipeg.ca> on 2004/06/24 09:18:22 UTC
[mp2] modperl_interp_unselect
Further to some problems I've been having with the current
cvs on Win32 (perl-5.8.4) and tests hanging (related to the
splitting off of APR::* from mod_perl.so), I think (?) this
can be traced to the workaround in xs/APR/APR/APR.xs on
defining modperl_interp_unselect:
==========================================================
#include "mod_perl.h"
/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
#ifndef modperl_interp_unselect apr_status_t
modperl_interp_unselect(void *data); apr_status_t
modperl_interp_unselect(void *data) { return APR_SUCCESS; }
#endif
===================================================================
Looking at how modperl_interp_select is defined in
src/modules/perl/modperl_interp.c, I tried the following
variation:
==================================================================
#include "mod_perl.h"
#include "modperl_common_util.h"
/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
#ifndef modperl_interp_unselect
apr_status_t modperl_interp_unselect(modperl_interp_t *interp) {
if (interp->refcnt != 0) {
--interp->refcnt;
}
return APR_SUCCESS; }
#endif
==================================================================
(I know I shouldn't be including mod_perl.h, but I did that
just to quickly define modperl_interp_t). In any case, this
seems to fix the problems, and all the tests pass for me,
including the apr-ext ones.
Does this fix seem kosher? Or some variation? I see from
src/modules/perl/modperl_types.h that modperl_interp_t
contains a request_rec member; is it legitimate to use
this structure when splitting APR::* from mod_perl.so
if one doesn't use the request_rec in this context?
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Thu, 24 Jun 2004, Joe Schaefer wrote:
> Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
>
> [...]
>
> > ==================================================================
> > #include "mod_perl.h"
> > #include "modperl_common_util.h"
> >
> > /* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
> > #ifndef modperl_interp_unselect
> > apr_status_t modperl_interp_unselect(modperl_interp_t *interp) {
>
>
> This has a different signature from the one in modperl_common_util.h,
> so you probably want
>
> apr_status_t modperl_interp_unselect(void *data) {
> modperl_interp_t *interp = data;
>
> [...]
That's a good point - here's a more complete implementation,
that also differentiates between USE_ITHREADS being defined
or not:
====================================================
Index: xs/APR/APR/APR.xs
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/APR/APR.xs,v
retrieving revision 1.11
diff -u -r1.11 APR.xs
--- xs/APR/APR/APR.xs 16 Jun 2004 03:55:48 -0000 1.11
+++ xs/APR/APR/APR.xs 24 Jun 2004 17:09:57 -0000
@@ -18,8 +18,17 @@
/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
#ifndef modperl_interp_unselect
apr_status_t modperl_interp_unselect(void *data);
-apr_status_t modperl_interp_unselect(void *data) { return APR_SUCCESS; }
-#endif
+apr_status_t modperl_interp_unselect(void *data)
+{
+#ifdef USE_ITHREADS
+ modperl_interp_t *interp = (modperl_interp_t *)data;
+ if (interp->refcnt != 0) {
+ --interp->refcnt;
+ }
+#endif /* USE_ITHREADS */
+ return APR_SUCCESS;
+}
+#endif /* modperl_interp_unselect */
#ifdef MP_HAVE_APR_LIBS
# define APR_initialize apr_initialize
================================================================
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
> On Thu, 24 Jun 2004, Stas Bekman wrote:
>
> [ ... ]
>
>>But which of the apr-ext tests happen to select
>>interpreters at all? Can you find out why did you need
>>that workaround. It's quite possible that your patch is
>>the right thing to do, but I made it a no-op since I
>>didn't see why would you want to unselect it at all.
>
>
> Actually, without the workaround, the apr-ext tests all pass
> for me, when run by themselves. The problem arose in the
> other tests - running 'nmake test' first encounters a
> problem with t/apache/discard_rbody hanging. But others
> hang as well when the order of the tests is changed.
>
>
>>I suppose the issue comes from loading APR.so and
>>mod_perl.so at the same time and having the wrong symbol
>>kicking in. It should have picked mod_perl.so's
>>modperl_interp_unselect, not APR.so's one.
>
>
> I think that's what's happening - it's picking up APR.so's
> modperl_interp_unselect everywhere for all APR/APR::*
> modules. This is because I did away with linking to
> mod_perl.so for all APR::* modules, including APR itself -
> if not, if mod_perl.so provides any symbols at all at link
> time, using APR and/or APR::* will demand that mod_perl.so
> be available.
What happens if in the future we will need to provide different functionality
for the same function one residing in APR.so and the other in mod_perl.so?
There is no way you can avoid linking mod_perl.so against APR.so?
>>If you can't control that, the right solution is to move
>>modperl_interp_unselect into the common area, instead of
>>duplicating it. Notice that I added that workaround
>>before we did the split, so it's probably the right thing
>>to do in any case.
>
>
> Just so I understand correctly, does that mean to remove
> modperl_interp_unselect from both xs/APR/APR/APR.xs and
> src/modules/perl/modperl_interp.c, and instead put in
> ===================================================================
> apr_status_t modperl_interp_unselect(void *data)
> {
> #ifdef USE_ITHREADS
> modperl_interp_t *interp = (modperl_interp_t *)data;
> if (interp->refcnt != 0) {
> --interp->refcnt;
> }
> #endif /* USE_ITHREADS */
> return APR_SUCCESS;
> }
> #endif /* modperl_interp_unselect */
> ===================================================================
> into something like src/modules/perl/modperl_common_util.c?
> And since this needs modperl_interp_t, move the declaration
> of this from src/modules/perl/modperl_types.h into
> src/modules/perl/modperl_common_types.h?
doh! you are right, there aren't the same. if you copy modperl_interp_unselect
from modperl_interp.c you will create a dependency on mod_perl.so.
I think the proper solution might be to rework the code that uses unselect to
call that function only if it figures out it's running inside Apache? I
haven't looked at how complex that would be.
> I noticed that modperl_interp_unselect is also used within
> src/modules/perl/mod_perl.c - if the above is correct, will
> that change affect anything there?
why should the move affect anything? it'll be still available as before.
Another concern (unrelated to this issue) is that we are now messing the
mapping of API groups within the files they are implemented in. I suppose it
needs to go into modperl_common_interp.[ch] to keep in sync.
Also may be we should replace /^modperl_common/ with some shorter name.
Ideally that prefix would be /^apr_/, but since quite a few of those things
aren't really coming from APR that choice may be confusing.
We don't have to shuffle the files/names now, besides layout out a plan and
finish the split first and have things working.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Thu, 24 Jun 2004, Stas Bekman wrote:
[ ... ]
> But which of the apr-ext tests happen to select
> interpreters at all? Can you find out why did you need
> that workaround. It's quite possible that your patch is
> the right thing to do, but I made it a no-op since I
> didn't see why would you want to unselect it at all.
Actually, without the workaround, the apr-ext tests all pass
for me, when run by themselves. The problem arose in the
other tests - running 'nmake test' first encounters a
problem with t/apache/discard_rbody hanging. But others
hang as well when the order of the tests is changed.
> I suppose the issue comes from loading APR.so and
> mod_perl.so at the same time and having the wrong symbol
> kicking in. It should have picked mod_perl.so's
> modperl_interp_unselect, not APR.so's one.
I think that's what's happening - it's picking up APR.so's
modperl_interp_unselect everywhere for all APR/APR::*
modules. This is because I did away with linking to
mod_perl.so for all APR::* modules, including APR itself -
if not, if mod_perl.so provides any symbols at all at link
time, using APR and/or APR::* will demand that mod_perl.so
be available.
> If you can't control that, the right solution is to move
> modperl_interp_unselect into the common area, instead of
> duplicating it. Notice that I added that workaround
> before we did the split, so it's probably the right thing
> to do in any case.
Just so I understand correctly, does that mean to remove
modperl_interp_unselect from both xs/APR/APR/APR.xs and
src/modules/perl/modperl_interp.c, and instead put in
===================================================================
apr_status_t modperl_interp_unselect(void *data)
{
#ifdef USE_ITHREADS
modperl_interp_t *interp = (modperl_interp_t *)data;
if (interp->refcnt != 0) {
--interp->refcnt;
}
#endif /* USE_ITHREADS */
return APR_SUCCESS;
}
#endif /* modperl_interp_unselect */
===================================================================
into something like src/modules/perl/modperl_common_util.c?
And since this needs modperl_interp_t, move the declaration
of this from src/modules/perl/modperl_types.h into
src/modules/perl/modperl_common_types.h?
I noticed that modperl_interp_unselect is also used within
src/modules/perl/mod_perl.c - if the above is correct, will
that change affect anything there?
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: [mp2] modperl_interp_unselect
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
> On Fri, 25 Jun 2004, Stas Bekman wrote:
>
>
>>Randy Kobes wrote:
>>
>>>On Thu, 24 Jun 2004, Joe Schaefer wrote:
>>>
>>>
>>>>Joe Schaefer <jo...@sunstarsys.com> writes:
>>>>
>>>>
>>>>>Perhaps using APR_OPTIONAL_FN* will do the trick here?
>>>>
>>>>Proof-of-concept patch below.
>
> [ .. ]
>
>>neat! Great idea Joe!
>>
>>
>>>+ (void)mpopt_interp_unselect(cdata->interp);
>>
>>but please use modperl_opt_ prefix if that's OK with you.
>
>
> Here's another version of the patch with the prefix changed.
> It also adds an #ifdef USE_ITHREADS/#endif around the
> APR_REGISTER_OPTIONAL_FN(modperl_interp_unselect) of
> mod_perl.c, which I needed on my (non-multi-thread) linux.
+1
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: [mp2] modperl_interp_unselect
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 25 Jun 2004, Stas Bekman wrote:
> Randy Kobes wrote:
> > On Thu, 24 Jun 2004, Joe Schaefer wrote:
> >
> >>Joe Schaefer <jo...@sunstarsys.com> writes:
> >>
> >>>Perhaps using APR_OPTIONAL_FN* will do the trick here?
> >>
> >>Proof-of-concept patch below.
[ .. ]
> neat! Great idea Joe!
>
> > + (void)mpopt_interp_unselect(cdata->interp);
>
> but please use modperl_opt_ prefix if that's OK with you.
Here's another version of the patch with the prefix changed.
It also adds an #ifdef USE_ITHREADS/#endif around the
APR_REGISTER_OPTIONAL_FN(modperl_interp_unselect) of
mod_perl.c, which I needed on my (non-multi-thread) linux.
=========================================================
? s.txt
Index: lib/ModPerl/WrapXS.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/WrapXS.pm,v
retrieving revision 1.76
diff -u -r1.76 WrapXS.pm
--- lib/ModPerl/WrapXS.pm 23 Jun 2004 03:30:15 -0000 1.76
+++ lib/ModPerl/WrapXS.pm 25 Jun 2004 14:54:24 -0000
@@ -548,6 +548,10 @@
}
}
+ if ($module eq 'APR::Pool') {
+ print $fh " modperl_opt_interp_unselect = APR_RETRIEVE_OPTIONAL_FN(modperl_interp_unselect);\n\n";
+ }
+
close $fh;
}
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.214
diff -u -r1.214 mod_perl.c
--- src/modules/perl/mod_perl.c 2 Jun 2004 21:35:58 -0000 1.214
+++ src/modules/perl/mod_perl.c 25 Jun 2004 14:54:24 -0000
@@ -715,6 +715,11 @@
void modperl_register_hooks(apr_pool_t *p)
{
+
+#ifdef USE_ITHREADS
+ APR_REGISTER_OPTIONAL_FN(modperl_interp_unselect);
+#endif
+
/* for <IfDefine MODPERL2> and Apache->define("MODPERL2") */
*(char **)apr_array_push(ap_server_config_defines) =
apr_pstrdup(p, "MODPERL2");
Index: src/modules/perl/mod_perl.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.h,v
retrieving revision 1.66
diff -u -r1.66 mod_perl.h
--- src/modules/perl/mod_perl.h 16 Jun 2004 03:55:47 -0000 1.66
+++ src/modules/perl/mod_perl.h 25 Jun 2004 14:54:24 -0000
@@ -128,4 +128,6 @@
/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
#define MODPERL_HOOK_REALLY_REALLY_FIRST (-20)
+APR_DECLARE_OPTIONAL_FN(apr_status_t,modperl_interp_unselect,(void *));
+
#endif /* MOD_PERL_H */
Index: xs/APR/APR/APR.xs
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/APR/APR.xs,v
retrieving revision 1.11
diff -u -r1.11 APR.xs
--- xs/APR/APR/APR.xs 16 Jun 2004 03:55:48 -0000 1.11
+++ xs/APR/APR/APR.xs 25 Jun 2004 14:54:24 -0000
@@ -15,12 +15,6 @@
#include "mod_perl.h"
-/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
-#ifndef modperl_interp_unselect
-apr_status_t modperl_interp_unselect(void *data);
-apr_status_t modperl_interp_unselect(void *data) { return APR_SUCCESS; }
-#endif
-
#ifdef MP_HAVE_APR_LIBS
# define APR_initialize apr_initialize
# define APR_terminate apr_terminate
Index: xs/APR/Pool/APR__Pool.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
retrieving revision 1.14
diff -u -r1.14 APR__Pool.h
--- xs/APR/Pool/APR__Pool.h 14 May 2004 07:40:31 -0000 1.14
+++ xs/APR/Pool/APR__Pool.h 25 Jun 2004 14:54:24 -0000
@@ -202,6 +202,9 @@
* callback wrapper for Perl cleanup subroutines
* @param data internal storage
*/
+
+static APR_OPTIONAL_FN_TYPE(modperl_interp_unselect) *modperl_opt_interp_unselect;
+
static apr_status_t mpxs_cleanup_run(void *data)
{
int count;
@@ -233,12 +236,12 @@
}
#ifdef USE_ITHREADS
- if (cdata->interp) {
+ if (cdata->interp && modperl_opt_interp_unselect) {
/* this will decrement the interp refcnt until
* there are no more references, in which case
* the interpreter will be putback into the mip
*/
- (void)modperl_interp_unselect(cdata->interp);
+ (void)modperl_opt_interp_unselect(cdata->interp);
}
#endif
===================================================================
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: [mp2] modperl_interp_unselect
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
>
> neat! Great idea Joe!
>
> > + (void)mpopt_interp_unselect(cdata->interp);
>
> but please use modperl_opt_ prefix if that's OK with you.
Sure- I put no thought into the prefix whatsoever. Eventually
I'll get a better feel for the way prefixes are organized in
mp2.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: [mp2] modperl_interp_unselect
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
> On Thu, 24 Jun 2004, Joe Schaefer wrote:
>
>
>>Joe Schaefer <jo...@sunstarsys.com> writes:
>>
>>
>>>Perhaps using APR_OPTIONAL_FN* will do the trick here?
>>
>>Proof-of-concept patch below. I don't know how to produce
>>a BOOT stanza in WrapXS/APR/Pool/Pool.xs, so after applying this patch
>>you also need to edit that file so the bottom looks like so
>
> [ .. ]
>
> Works great for me - thanks, Joe! Here's a copy of the patch
> together with a small addition to lib/ModPerl/WrapXS.pm
> which makes that change mentioned above to the end of
> APR/APR/Pool/Pool.xs.
neat! Great idea Joe!
> + (void)mpopt_interp_unselect(cdata->interp);
but please use modperl_opt_ prefix if that's OK with you.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: [mp2] modperl_interp_unselect
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Thu, 24 Jun 2004, Joe Schaefer wrote:
> Joe Schaefer <jo...@sunstarsys.com> writes:
>
> > Perhaps using APR_OPTIONAL_FN* will do the trick here?
>
> Proof-of-concept patch below. I don't know how to produce
> a BOOT stanza in WrapXS/APR/Pool/Pool.xs, so after applying this patch
> you also need to edit that file so the bottom looks like so
[ .. ]
Works great for me - thanks, Joe! Here's a copy of the patch
together with a small addition to lib/ModPerl/WrapXS.pm
which makes that change mentioned above to the end of
APR/APR/Pool/Pool.xs.
=======================================================================
Index: lib/ModPerl/WrapXS.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/WrapXS.pm,v
retrieving revision 1.76
diff -u -r1.76 WrapXS.pm
--- lib/ModPerl/WrapXS.pm 23 Jun 2004 03:30:15 -0000 1.76
+++ lib/ModPerl/WrapXS.pm 25 Jun 2004 02:42:32 -0000
@@ -548,6 +548,10 @@
}
}
+ if ($module eq 'APR::Pool') {
+ print $fh " mpopt_interp_unselect = APR_RETRIEVE_OPTIONAL_FN(modperl_interp_unselect);\n\n";
+ }
+
close $fh;
}
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.214
diff -u -r1.214 mod_perl.c
--- src/modules/perl/mod_perl.c 2 Jun 2004 21:35:58 -0000 1.214
+++ src/modules/perl/mod_perl.c 25 Jun 2004 02:42:32 -0000
@@ -715,6 +715,8 @@
void modperl_register_hooks(apr_pool_t *p)
{
+ APR_REGISTER_OPTIONAL_FN(modperl_interp_unselect);
+
/* for <IfDefine MODPERL2> and Apache->define("MODPERL2") */
*(char **)apr_array_push(ap_server_config_defines) =
apr_pstrdup(p, "MODPERL2");
Index: src/modules/perl/mod_perl.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.h,v
retrieving revision 1.66
diff -u -r1.66 mod_perl.h
--- src/modules/perl/mod_perl.h 16 Jun 2004 03:55:47 -0000 1.66
+++ src/modules/perl/mod_perl.h 25 Jun 2004 02:42:32 -0000
@@ -128,4 +128,6 @@
/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
#define MODPERL_HOOK_REALLY_REALLY_FIRST (-20)
+APR_DECLARE_OPTIONAL_FN(apr_status_t,modperl_interp_unselect,(void *));
+
#endif /* MOD_PERL_H */
Index: xs/APR/APR/APR.xs
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/APR/APR.xs,v
retrieving revision 1.11
diff -u -r1.11 APR.xs
--- xs/APR/APR/APR.xs 16 Jun 2004 03:55:48 -0000 1.11
+++ xs/APR/APR/APR.xs 25 Jun 2004 02:42:32 -0000
@@ -15,12 +15,6 @@
#include "mod_perl.h"
-/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
-#ifndef modperl_interp_unselect
-apr_status_t modperl_interp_unselect(void *data);
-apr_status_t modperl_interp_unselect(void *data) { return APR_SUCCESS; }
-#endif
-
#ifdef MP_HAVE_APR_LIBS
# define APR_initialize apr_initialize
# define APR_terminate apr_terminate
Index: xs/APR/Pool/APR__Pool.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
retrieving revision 1.14
diff -u -r1.14 APR__Pool.h
--- xs/APR/Pool/APR__Pool.h 14 May 2004 07:40:31 -0000 1.14
+++ xs/APR/Pool/APR__Pool.h 25 Jun 2004 02:42:32 -0000
@@ -202,6 +202,9 @@
* callback wrapper for Perl cleanup subroutines
* @param data internal storage
*/
+
+static APR_OPTIONAL_FN_TYPE(modperl_interp_unselect) *mpopt_interp_unselect;
+
static apr_status_t mpxs_cleanup_run(void *data)
{
int count;
@@ -233,12 +236,12 @@
}
#ifdef USE_ITHREADS
- if (cdata->interp) {
+ if (cdata->interp && mpopt_interp_unselect) {
/* this will decrement the interp refcnt until
* there are no more references, in which case
* the interpreter will be putback into the mip
*/
- (void)modperl_interp_unselect(cdata->interp);
+ (void)mpopt_interp_unselect(cdata->interp);
}
#endif
================================================================
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: [mp2] modperl_interp_unselect
Posted by Joe Schaefer <jo...@sunstarsys.com>.
[Oops, I missed the APR__Pool.h section of the patch!
please disregard the previous post, this is a duplicate
with the full patch]
Joe Schaefer <jo...@sunstarsys.com> writes:
> Perhaps using APR_OPTIONAL_FN* will do the trick here?
Proof-of-concept patch below. I don't know how to produce
a BOOT stanza in WrapXS/APR/Pool/Pool.xs, so after applying this patch
you also need to edit that file so the bottom looks like so
% tail WrapXS/APR/Pool/Pool.xs
mpxs_apr_pool_DESTROY(aTHX_ obj);
MODULE = APR::Pool
PROTOTYPES: disabled
BOOT:
items = items; /* -Wall */
mpopt_interp_unselect = APR_RETRIEVE_OPTIONAL_FN(modperl_interp_unselect);
%
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.214
diff -u -r1.214 mod_perl.c
--- src/modules/perl/mod_perl.c 2 Jun 2004 21:35:58 -0000 1.214
+++ src/modules/perl/mod_perl.c 24 Jun 2004 20:10:06 -0000
@@ -715,6 +715,8 @@
void modperl_register_hooks(apr_pool_t *p)
{
+ APR_REGISTER_OPTIONAL_FN(modperl_interp_unselect);
+
/* for <IfDefine MODPERL2> and Apache->define("MODPERL2") */
*(char **)apr_array_push(ap_server_config_defines) =
apr_pstrdup(p, "MODPERL2");
Index: src/modules/perl/mod_perl.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.h,v
retrieving revision 1.66
diff -u -r1.66 mod_perl.h
--- src/modules/perl/mod_perl.h 16 Jun 2004 03:55:47 -0000 1.66
+++ src/modules/perl/mod_perl.h 24 Jun 2004 20:10:06 -0000
@@ -128,4 +128,6 @@
/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
#define MODPERL_HOOK_REALLY_REALLY_FIRST (-20)
+APR_DECLARE_OPTIONAL_FN(apr_status_t,modperl_interp_unselect,(void *));
+
#endif /* MOD_PERL_H */
cvs server: Diffing xs/APR
cvs server: Diffing xs/APR/APR
Index: xs/APR/APR/APR.xs
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/APR/APR/APR.xs,v
retrieving revision 1.11
diff -u -r1.11 APR.xs
--- xs/APR/APR/APR.xs 16 Jun 2004 03:55:48 -0000 1.11
+++ xs/APR/APR/APR.xs 24 Jun 2004 20:10:06 -0000
@@ -15,12 +15,6 @@
#include "mod_perl.h"
-/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
-#ifndef modperl_interp_unselect
-apr_status_t modperl_interp_unselect(void *data);
-apr_status_t modperl_interp_unselect(void *data) { return APR_SUCCESS; }
-#endif
-
#ifdef MP_HAVE_APR_LIBS
# define APR_initialize apr_initialize
# define APR_terminate apr_terminate
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
retrieving revision 1.14
diff -u -r1.14 APR__Pool.h
--- xs/APR/Pool/APR__Pool.h 14 May 2004 07:40:31 -0000 1.14
+++ xs/APR/Pool/APR__Pool.h 24 Jun 2004 20:10:07 -0000
@@ -202,6 +202,9 @@
* callback wrapper for Perl cleanup subroutines
* @param data internal storage
*/
+
+static APR_OPTIONAL_FN_TYPE(modperl_interp_unselect) *mpopt_interp_unselect;
+
static apr_status_t mpxs_cleanup_run(void *data)
{
int count;
@@ -233,12 +236,12 @@
}
#ifdef USE_ITHREADS
- if (cdata->interp) {
+ if (cdata->interp && mpopt_interp_unselect) {
/* this will decrement the interp refcnt until
* there are no more references, in which case
* the interpreter will be putback into the mip
*/
- (void)modperl_interp_unselect(cdata->interp);
+ (void)mpopt_interp_unselect(cdata->interp);
}
#endif
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
[PATCH] Re: [mp2] modperl_interp_unselect
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
> Perhaps using APR_OPTIONAL_FN* will do the trick here?
Proof-of-concept patch below. I don't know how to produce
a BOOT stanza in WrapXS/APR/Pool/Pool.xs, so after applying this patch
you also need to edit that file so the bottom looks like so
% tail WrapXS/APR/Pool/Pool.xs
mpxs_apr_pool_DESTROY(aTHX_ obj);
MODULE = APR::Pool
PROTOTYPES: disabled
BOOT:
items = items; /* -Wall */
mpopt_interp_unselect = APR_RETRIEVE_OPTIONAL_FN(modperl_interp_unselect);
%
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.214
diff -u -r1.214 mod_perl.c
--- src/modules/perl/mod_perl.c 2 Jun 2004 21:35:58 -0000 1.214
+++ src/modules/perl/mod_perl.c 24 Jun 2004 20:10:06 -0000
@@ -715,6 +715,8 @@
void modperl_register_hooks(apr_pool_t *p)
{
+ APR_REGISTER_OPTIONAL_FN(modperl_interp_unselect);
+
/* for <IfDefine MODPERL2> and Apache->define("MODPERL2") */
*(char **)apr_array_push(ap_server_config_defines) =
apr_pstrdup(p, "MODPERL2");
Index: src/modules/perl/mod_perl.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.h,v
retrieving revision 1.66
diff -u -r1.66 mod_perl.h
--- src/modules/perl/mod_perl.h 16 Jun 2004 03:55:47 -0000 1.66
+++ src/modules/perl/mod_perl.h 24 Jun 2004 20:10:06 -0000
@@ -128,4 +128,6 @@
/* we need to hook a few internal things before APR_HOOK_REALLY_FIRST */
#define MODPERL_HOOK_REALLY_REALLY_FIRST (-20)
+APR_DECLARE_OPTIONAL_FN(apr_status_t,modperl_interp_unselect,(void *));
+
#endif /* MOD_PERL_H */
cvs server: Diffing xs/APR
cvs server: Diffing xs/APR/APR
Index: xs/APR/APR/APR.xs
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/APR/APR/APR.xs,v
retrieving revision 1.11
diff -u -r1.11 APR.xs
--- xs/APR/APR/APR.xs 16 Jun 2004 03:55:48 -0000 1.11
+++ xs/APR/APR/APR.xs 24 Jun 2004 20:10:06 -0000
@@ -15,12 +15,6 @@
#include "mod_perl.h"
-/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
-#ifndef modperl_interp_unselect
-apr_status_t modperl_interp_unselect(void *data);
-apr_status_t modperl_interp_unselect(void *data) { return APR_SUCCESS; }
-#endif
-
#ifdef MP_HAVE_APR_LIBS
# define APR_initialize apr_initialize
# define APR_terminate apr_terminate
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
[...]
> That's a good idea - I'll try that tonight, in a few hours,
> when I have access to my Win32 machine. But I'm 99% sure
> (meaning there's a 50% chance I'm wrong :) that APR/APR::*
> are using APR.so's modperl_interp_unselect. The reason for
> this is that, on Win32, the symbols specified at link time
> are the ones used, and I took out all references to the
> mod_perl lib when linking APR/APR::*. So even when
> mod_perl.so is loaded in a test, the modperl_interp_unselect
> symbol present there is ignored by APR/APR::*.
Right, I think we're too ELF-dependent currently,
which is why we changed to a function-pointer approach
in apreq_env. However apreq2's advantage over mp2 here
is that there's a shared library libapreq2 which holds
the function pointer reference. Everything in mp2 is
a module, so I don't think it's possible to go that route
here.
Perhaps using APR_OPTIONAL_FN* will do the trick here?
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Thu, 24 Jun 2004, Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
> > I suppose the issue comes from loading APR.so and mod_perl.so at the
> > same time and having the wrong symbol kicking in. It should have
> > picked mod_perl.so's modperl_interp_unselect, not APR.so's one.
>
> This is my concern, too, because on my box, the APR version of
> modperl_interp_unselect is never actually invoked by the test suite.
> Randy, throw in an abort() call like this and see if you get any segfaults
> with the tests.
>
>
> #include <stdlib.h>
>
> /* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
> #ifndef modperl_interp_unselect
> apr_status_t modperl_interp_unselect(void *data) {
> modperl_interp_t *interp = data;
>
> abort(); /* GOT HERE on core dump */
>
> #ifdef USE_ITHREADS
> if (interp->refcnt != 0) {
> --interp->refcnt;
> }
> #endif
> return APR_SUCCESS;
> }
> #endif
That's a good idea - I'll try that tonight, in a few hours,
when I have access to my Win32 machine. But I'm 99% sure
(meaning there's a 50% chance I'm wrong :) that APR/APR::*
are using APR.so's modperl_interp_unselect. The reason for
this is that, on Win32, the symbols specified at link time
are the ones used, and I took out all references to the
mod_perl lib when linking APR/APR::*. So even when
mod_perl.so is loaded in a test, the modperl_interp_unselect
symbol present there is ignored by APR/APR::*.
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> I suppose the issue comes from loading APR.so and mod_perl.so at the
> same time and having the wrong symbol kicking in. It should have
> picked mod_perl.so's modperl_interp_unselect, not APR.so's one.
This is my concern, too, because on my box, the APR version of
modperl_interp_unselect is never actually invoked by the test suite.
Randy, throw in an abort() call like this and see if you get any segfaults
with the tests.
#include <stdlib.h>
/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
#ifndef modperl_interp_unselect
apr_status_t modperl_interp_unselect(void *data) {
modperl_interp_t *interp = data;
abort(); /* GOT HERE on core dump */
#ifdef USE_ITHREADS
if (interp->refcnt != 0) {
--interp->refcnt;
}
#endif
return APR_SUCCESS;
}
#endif
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
>
> [...]
>
>
>>==================================================================
>>#include "mod_perl.h"
>>#include "modperl_common_util.h"
>>
>>/* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
>>#ifndef modperl_interp_unselect
>>apr_status_t modperl_interp_unselect(modperl_interp_t *interp) {
>
>
>
> This has a different signature from the one in modperl_common_util.h,
> so you probably want
>
> apr_status_t modperl_interp_unselect(void *data) {
> modperl_interp_t *interp = data;
>
> [...]
>
>
>>(I know I shouldn't be including mod_perl.h, but I did that
>>just to quickly define modperl_interp_t). In any case, this
>>seems to fix the problems, and all the tests pass for me,
>>including the apr-ext ones.
>
>
> I don't notice any change on debian-amd64 - all tests pass either
> way (except t/protocol/echo_timeout.t, which is expected since
> I'm testing against HEAD for apr & httpd-2.0). However, given
> that mpxs_apr_pool_cleanup_register increments refcnt, your
> implementation seems to be an improvement over current-cvs.
But which of the apr-ext tests happen to select interpreters at all? Can you
find out why did you need that workaround. It's quite possible that your patch
is the right thing to do, but I made it a no-op since I didn't see why would
you want to unselect it at all.
I suppose the issue comes from loading APR.so and mod_perl.so at the same time
and having the wrong symbol kicking in. It should have picked mod_perl.so's
modperl_interp_unselect, not APR.so's one.
If you can't control that, the right solution is to move
modperl_interp_unselect into the common area, instead of duplicating it.
Notice that I added that workaround before we did the split, so it's probably
the right thing to do in any case.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] modperl_interp_unselect
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
[...]
> ==================================================================
> #include "mod_perl.h"
> #include "modperl_common_util.h"
>
> /* XXX: provide the missing symbol for APR::Pool as a tmp workaround */
> #ifndef modperl_interp_unselect
> apr_status_t modperl_interp_unselect(modperl_interp_t *interp) {
This has a different signature from the one in modperl_common_util.h,
so you probably want
apr_status_t modperl_interp_unselect(void *data) {
modperl_interp_t *interp = data;
[...]
> (I know I shouldn't be including mod_perl.h, but I did that
> just to quickly define modperl_interp_t). In any case, this
> seems to fix the problems, and all the tests pass for me,
> including the apr-ext ones.
I don't notice any change on debian-amd64 - all tests pass either
way (except t/protocol/echo_timeout.t, which is expected since
I'm testing against HEAD for apr & httpd-2.0). However, given
that mpxs_apr_pool_cleanup_register increments refcnt, your
implementation seems to be an improvement over current-cvs.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org