Skip Menu |
 

This queue is for tickets about the mod_perl CPAN distribution.

Report information
The Basics
Id: 101962
Status: resolved
Priority: 0/
Queue: mod_perl

People
Owner: Nobody in particular
Requestors: shay [...] cpan.org
Cc: abe+cpan [...] deuxchevaux.org
dam [...] cpan.org
dom [...] cpan.org
ether [...] cpan.org
gregoa [...] cpan.org
PLICEASE [...] cpan.org
AdminCc:

Bug Information
Severity: (no value)
Broken in: 2.0.9
Fixed in: 2.0.10



Subject: modperl_env_init() needs changing for perl >= 5.21.8
The following commit causes mod_perl (svn trunk) to fail on startup (on Windows, at least) using 5.21.8: http://perl5.git.perl.org/perl.git/commit/c910fead78 It works fine on the same system (VC++ 2010, with httpd-2.4.10) using 5.21.7. I get an access violation in modperl_env_init() when trying to replace the now const PL_vtbls with its own versions: void modperl_env_init(void) { /* save originals */ StructCopy(&PL_vtbl_env, &MP_PERL_vtbl_env, MGVTBL); StructCopy(&PL_vtbl_envelem, &MP_PERL_vtbl_envelem, MGVTBL); /* replace with our versions */ StructCopy(&MP_vtbl_env, &PL_vtbl_env, MGVTBL); StructCopy(&MP_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); } I reported this in https://rt.perl.org/Ticket/Display.html?id=123687 but (as I suspected) this was deemed to be a mod_perl problem that we need to resolve, not a bug in perl. In that ticket, Leon suggested, "lookup %ENV's magic, and then replace the pointer to PL_vtbl_env with a pointer to MP_vtbl_env. You may have to add some svt_copy magic to make it cast MP_vtbl_envelem instead of PL_vtbl_envelem on the elements."
Download (untitled) / with headers
text/plain 351b
On Fri Feb 06 03:23:10 2015, SHAY wrote: Show quoted text
> The following commit causes mod_perl (svn trunk) to fail on startup > (on Windows, at least) using 5.21.8: > > http://perl5.git.perl.org/perl.git/commit/c910fead78 >
The same has also been seen elsewhere, so this is not a Windows-specific problem: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=787493
Download (untitled) / with headers
text/plain 660b
On Thu Jun 04 08:25:33 2015, SHAY wrote: Show quoted text
> On Fri Feb 06 03:23:10 2015, SHAY wrote:
> > The following commit causes mod_perl (svn trunk) to fail on startup > > (on Windows, at least) using 5.21.8: > > > > http://perl5.git.perl.org/perl.git/commit/c910fead78 > >
> > The same has also been seen elsewhere, so this is not a Windows- > specific problem: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=787493
Hello Steve, I wondered if you had any plans to address this? It's going to be a blocker for perl 5.22 in Debian quite soon. I'm afraid I can't help myself as I don't have the necessary expertise. Thanks, Dominic (Debian perl maintenance team)
Download (untitled) / with headers
text/plain 434b
Le Dim 16 Aoû 2015 12:28:06, DOM a écrit : Show quoted text
> I wondered if you had any plans to address this? It's going to be a > blocker for perl 5.22 in Debian quite soon. I'm afraid I can't help > myself as I don't have the necessary expertise.
Hi, It is also going to be a problem on FreeBSD soon, our plan was to switch the default to 5.22 Perl version in September, and switch to the new Perl release every September from now on. Thanks,
Download (untitled) / with headers
text/plain 974b
On Sun Aug 16 06:28:06 2015, DOM wrote: Show quoted text
> On Thu Jun 04 08:25:33 2015, SHAY wrote:
> > On Fri Feb 06 03:23:10 2015, SHAY wrote:
> > > The following commit causes mod_perl (svn trunk) to fail on startup > > > (on Windows, at least) using 5.21.8: > > > > > > http://perl5.git.perl.org/perl.git/commit/c910fead78 > > >
> > > > The same has also been seen elsewhere, so this is not a Windows- > > specific problem: > > > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=787493
> > Hello Steve, > > I wondered if you had any plans to address this? It's going to be a > blocker for perl 5.22 in Debian quite soon. I'm afraid I can't help > myself as I don't have the necessary expertise. >
Sorry for the slow response. I'm busy with a couple of Perl maint releases at the moment but I do plan to get to this next if nobody else has done it by then. I haven't looked at it very much yet, but I'm hoping that the plan outlined by Leon shouldn't be too difficult to implement.
Download (untitled) / with headers
text/plain 1.1k
On Mon Aug 31 12:16:46 2015, SHAY wrote: Show quoted text
> On Sun Aug 16 06:28:06 2015, DOM wrote:
> > On Thu Jun 04 08:25:33 2015, SHAY wrote:
> > > On Fri Feb 06 03:23:10 2015, SHAY wrote:
> > > > The following commit causes mod_perl (svn trunk) to fail on > > > > startup > > > > (on Windows, at least) using 5.21.8: > > > > > > > > http://perl5.git.perl.org/perl.git/commit/c910fead78 > > > >
> > > > > > The same has also been seen elsewhere, so this is not a Windows- > > > specific problem: > > > > > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=787493
> > > > Hello Steve, > > > > I wondered if you had any plans to address this? It's going to be a > > blocker for perl 5.22 in Debian quite soon. I'm afraid I can't help > > myself as I don't have the necessary expertise. > >
> > Sorry for the slow response. I'm busy with a couple of Perl maint > releases at the moment but I do plan to get to this next if nobody > else has done it by then. I haven't looked at it very much yet, but > I'm hoping that the plan outlined by Leon shouldn't be too difficult > to implement.
That's great, thanks! Do shout if you need help testing. Cheers, Dominic.
Subject: [rt.cpan.org #101962]
Date: Thu, 1 Oct 2015 05:35:03 -0400
To: bug-mod_perl [...] rt.cpan.org
From: Chris Kaltwasser <chris [...] netpos.com>
Download (untitled) / with headers
text/plain 1.2k
Hello, A recent post by DOM regarding the mod_perl bug report 101962 indicates that a fix might be forth coming. I'm hoping to see if I can obtain that fix as I'm a bit stuck given my attempt to use mod_perl on my Fedora 22 development environment. I turns out I'm running into the same bug reported in that bug concerning mod_perl initialization using Fedora 22. I have compiled my perl and apache stack in debug mode and recived a core dump while performing the make test for mod_perl. I also get the same core dump later when trying to start apache. The debugger indications the failure is at the exact point discussed in this bug report. Indicated below. void modperl_env_init(void) { /* save originals */ StructCopy(&PL_vtbl_env, &MP_PERL_vtbl_env, MGVTBL); StructCopy(&PL_vtbl_envelem, &MP_PERL_vtbl_envelem, MGVTBL); /* replace with our versions */ --> StructCopy(&MP_vtbl_env, &PL_vtbl_env, MGVTBL); StructCopy(&MP_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); } Any assistance with this would be greatly appreciated!!! Thank you -- Show quoted text
____________________________________ Chris Kaltwasser <chris@netpos.com> Chief Technical Officer NetPOS LLC<http://netpos.com> fon: 734.302.4220 cel: 734.645.6215
____________________________________
From: ntyni [...] iki.fi
Download (untitled) / with headers
text/plain 809b
On Mon Aug 31 07:16:46 2015, SHAY wrote: Show quoted text
> I'm hoping that the plan outlined by Leon shouldn't be too difficult to implement.
Hi, I'm attaching a work in progress patch that tries to do this. I'm somewhat stuck here. Comments would be appreciated, but please be gentle - I'm quite out of my depth here and only learning the ropes as I go along :) Main open questions that I can see: - where should modperl_env_init2() be really called? It needs an initialized Perl interpreter for ENVHV (%ENV), so modperl_sys_init() seems too early - cleanup (replacement for modperl_env_unload()) is nonexistent so far - running t/modperl/env.t and t/modules/cgi.t in the same run makes the latter fail because $ENV{MOD_PERL} is not set Not sure if the modperl-dev list would be a better place to discuss this?
Subject: 0001-First-steps-at-Perl-5.22-compatibility.patch
From 8bbde4e27e2e38c98934f4b409ba1fb6b9ad1356 Mon Sep 17 00:00:00 2001 From: Niko Tyni <ntyni@debian.org> Date: Thu, 29 Oct 2015 00:18:53 +0200 Subject: [PATCH] First steps at Perl 5.22 compatibility As outlined by Leon Timmermans in [perl #123687]: lookup %ENV's magic, and then replace the pointer to PL_vtbl_env with a pointer to MP_vtbl_env. You may have to add some svt_copy magic to make it cast MP_vtbl_envelem instead of PL_vtbl_envelem on the elements. with an added svt_local for the 'local %ENV' tests. This is enough to make t/modperl/env.t pass, but is probably mostly incorrect. Main open questions: - where should modperl_env_init2() be really called? It needs an initialized Perl interpreter for ENVHV (%ENV), so modperl_sys_init() seems too early - cleanup (replacement for modperl_env_unload()) is nonexistent - running t/modperl/env.t and t/modules/cgi.t in the same run makes the latter fail because $ENV{MOD_PERL} is not set Bug: https://rt.cpan.org/Public/Bug/Display.html?id=101962 Bug: https://rt.perl.org/Ticket/Display.html?id=123687 --- src/modules/perl/modperl_env.c | 61 ++++++++++++++++++++++++++++++++++++++++++ src/modules/perl/modperl_env.h | 2 ++ 2 files changed, 63 insertions(+) diff --git a/src/modules/perl/modperl_env.c b/src/modules/perl/modperl_env.c index c1a276b..a912994 100644 --- a/src/modules/perl/modperl_env.c +++ b/src/modules/perl/modperl_env.c @@ -121,6 +121,9 @@ static void modperl_env_table_populate(pTHX_ apr_table_t *table) const apr_array_header_t *array; apr_table_entry_t *elts; +#if MP_PERL_VERSION_AT_LEAST(5, 21, 8) + modperl_env_init2(aTHX); +#endif modperl_env_untie(mg_flags); array = apr_table_elts(table); @@ -612,6 +615,9 @@ static int modperl_env_magic_get(pTHX_ SV *sv, MAGIC *mg) } #endif +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen); +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg); + /* override %ENV virtual tables with our own */ static MGVTBL MP_vtbl_env = { 0, @@ -619,6 +625,7 @@ static MGVTBL MP_vtbl_env = { 0, modperl_env_magic_clear_all, 0 + , modperl_env_magic_copy, 0, modperl_env_magic_local_all }; static MGVTBL MP_vtbl_envelem = { @@ -629,22 +636,76 @@ static MGVTBL MP_vtbl_envelem = { 0 }; +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen) +{ + MAGIC *nmg = NULL; + sv_magic(nsv, mg->mg_obj, + toLOWER(mg->mg_type), + name, namlen); + nmg = mg_find(nsv, toLOWER(mg->mg_type)); + if (mg->mg_virtual == &MP_vtbl_env) { + MP_TRACE_d(MP_FUNC, "copying magic to %%ENV element"); + nmg->mg_virtual = &MP_vtbl_envelem; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found to copy"); + } + return 1; +} + +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg) +{ + MAGIC *nmg; + MP_TRACE_e(MP_FUNC, "localizing %%ENV"); + sv_magic(nsv, mg->mg_obj, + mg->mg_type, + NULL, 0); + nmg = mg_find(nsv, mg->mg_type); + nmg->mg_ptr = mg->mg_ptr; + if (mg->mg_virtual == &MP_vtbl_env) { + nmg->mg_virtual = &MP_vtbl_env; + nmg->mg_flags |= MGf_COPY; + nmg->mg_flags |= MGf_LOCAL; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found to localize"); + } + return 1; +} + void modperl_env_init(void) { /* save originals */ StructCopy(&PL_vtbl_env, &MP_PERL_vtbl_env, MGVTBL); StructCopy(&PL_vtbl_envelem, &MP_PERL_vtbl_envelem, MGVTBL); +#if MP_PERL_VERSION_AT_MOST(5, 21, 7) /* replace with our versions */ StructCopy(&MP_vtbl_env, &PL_vtbl_env, MGVTBL); StructCopy(&MP_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); +#endif +} + +void modperl_env_init2(pTHX) +{ + MAGIC *mg; + + MP_TRACE_d(MP_FUNC, "changing vtable for %%ENV"); + mg = mg_find((SV *)ENVHV, PERL_MAGIC_env); + if (mg) { + mg->mg_virtual = &MP_vtbl_env; + mg->mg_flags |= MGf_COPY; + mg->mg_flags |= MGf_LOCAL; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found?"); + } } void modperl_env_unload(void) { +#if MP_PERL_VERSION_AT_MOST(5, 21, 7) /* restore originals */ StructCopy(&MP_PERL_vtbl_env, &PL_vtbl_env, MGVTBL); StructCopy(&MP_PERL_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); +#endif } /* diff --git a/src/modules/perl/modperl_env.h b/src/modules/perl/modperl_env.h index 406e345..d00ebe2 100644 --- a/src/modules/perl/modperl_env.h +++ b/src/modules/perl/modperl_env.h @@ -60,6 +60,8 @@ void modperl_env_request_untie(pTHX_ request_rec *r); void modperl_env_init(void); +void modperl_env_init2(pTHX); + void modperl_env_unload(void); #endif /* MODPERL_ENV_H */ -- 2.5.1
Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Wed, 28 Oct 2015 23:47:45 +0000
To: bug-mod_perl [...] rt.cpan.org
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 1.3k
On 28 Oct 2015 22:49, "ntyni@iki.fi via RT" <bug-mod_perl@rt.cpan.org> wrote: Show quoted text
> > Queue: mod_perl > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > On Mon Aug 31 07:16:46 2015, SHAY wrote: >
> > I'm hoping that the plan outlined by Leon shouldn't be too difficult to
implement. Show quoted text
> > Hi, I'm attaching a work in progress patch that tries to do this. I'm
somewhat stuck here. Comments would be appreciated, but please be gentle - I'm quite out of my depth here and only learning the ropes as I go along :) Show quoted text
> > > Main open questions that I can see: > > - where should modperl_env_init2() be really called? It needs an
initialized Perl interpreter for ENVHV (%ENV), so modperl_sys_init() seems too early Show quoted text
> > - cleanup (replacement for modperl_env_unload()) is nonexistent so far > > - running t/modperl/env.t and t/modules/cgi.t in the same run makes the
latter fail because $ENV{MOD_PERL} is not set Show quoted text
> > Not sure if the modperl-dev list would be a better place to discuss this? >
Hi, Oddly enough I've also been looking at this, and was asking Leon Timmermans for advice only just today! Attached is my effort so far, but I'm told that I've gone wrong with PERL_MAGIC_envelem and I need a mg_copy entry in MP_PERL_vtbl_env. I will try this tomorrow. Hopefully we can get this nailed soon between us :-)
Download env.patch
text/x-diff 3.5k

Message body is not shown because sender requested not to inline it.

Download (untitled) / with headers
text/plain 2.1k
On Wed Oct 28 19:47:56 2015, steve.m.hay@googlemail.com wrote: Show quoted text
> On 28 Oct 2015 22:49, "ntyni@iki.fi via RT" <bug-mod_perl@rt.cpan.org> > wrote:
> > > > Queue: mod_perl > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > > > On Mon Aug 31 07:16:46 2015, SHAY wrote: > >
> > > I'm hoping that the plan outlined by Leon shouldn't be too difficult to
> implement.
> > > > Hi, I'm attaching a work in progress patch that tries to do this. I'm
> somewhat stuck here. Comments would be appreciated, but please be gentle - > I'm quite out of my depth here and only learning the ropes as I go along :)
> > > > > > Main open questions that I can see: > > > > - where should modperl_env_init2() be really called? It needs an
> initialized Perl interpreter for ENVHV (%ENV), so modperl_sys_init() seems > too early
> > > > - cleanup (replacement for modperl_env_unload()) is nonexistent so far > > > > - running t/modperl/env.t and t/modules/cgi.t in the same run makes the
> latter fail because $ENV{MOD_PERL} is not set
> > > > Not sure if the modperl-dev list would be a better place to discuss this? > >
> > Hi, > > Oddly enough I've also been looking at this, and was asking Leon Timmermans > for advice only just today! > > Attached is my effort so far, but I'm told that I've gone wrong with > PERL_MAGIC_envelem and I need a mg_copy entry in MP_PERL_vtbl_env. I will > try this tomorrow. > > Hopefully we can get this nailed soon between us :-)
The mg_copy function in your patch looks just the ticket to me, but unfortunately I'm getting failures in t/modperl/setupenv2.t (tests 17-23) (in addition to the slew of failures which are now sadly the norm on Windows when using httpd-2.4). I've also tried combining the init/term changes from my patch (although I'm not sure whether they're really correct either) with the other good work of your patch. The result is attached (env2.patch). This fixes the setupenv2.t failures but has a new problem instead - in t/modperl/env.t (planned 71 tests but ran 6). I haven't looked into what the exact cause of either of those failures is yet. Hopefully doing so will shed some more light on this...
Subject: env2.patch
Download env2.patch
text/x-diff 4.8k
Index: src/modules/perl/mod_perl.c =================================================================== --- src/modules/perl/mod_perl.c (revision 1711313) +++ src/modules/perl/mod_perl.c (working copy) @@ -262,6 +262,8 @@ exit(1); } + modperl_env_init(aTHX); + /* suspend END blocks to be run at server shutdown */ endav = PL_endav; PL_endav = (AV *)NULL; @@ -576,9 +578,6 @@ /* modifies PL_ppaddr */ modperl_perl_pp_set_all(); - /* modifies PL_vtbl_env{elem} */ - modperl_env_init(); - return APR_SUCCESS; } @@ -597,8 +596,6 @@ MP_TRACE_i(MP_FUNC, "mod_perl sys term"); - modperl_env_unload(); - modperl_perl_pp_unset_all(); PERL_SYS_TERM(); Index: src/modules/perl/modperl_env.c =================================================================== --- src/modules/perl/modperl_env.c (revision 1711313) +++ src/modules/perl/modperl_env.c (working copy) @@ -612,6 +612,9 @@ } #endif +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen); +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg); + /* override %ENV virtual tables with our own */ static MGVTBL MP_vtbl_env = { 0, @@ -618,7 +621,10 @@ modperl_env_magic_set_all, 0, modperl_env_magic_clear_all, - 0 + 0, + modperl_env_magic_copy, + 0, + modperl_env_magic_local_all }; static MGVTBL MP_vtbl_envelem = { @@ -629,24 +635,77 @@ 0 }; -void modperl_env_init(void) +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen) { - /* save originals */ - StructCopy(&PL_vtbl_env, &MP_PERL_vtbl_env, MGVTBL); - StructCopy(&PL_vtbl_envelem, &MP_PERL_vtbl_envelem, MGVTBL); + MAGIC *nmg = NULL; + sv_magic(nsv, mg->mg_obj, + toLOWER(mg->mg_type), + name, namlen); + nmg = mg_find(nsv, toLOWER(mg->mg_type)); + if (mg->mg_virtual == &MP_vtbl_env) { + MP_TRACE_d(MP_FUNC, "copying magic to %%ENV element"); + nmg->mg_virtual = &MP_vtbl_envelem; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found to copy"); + } + return 1; +} - /* replace with our versions */ - StructCopy(&MP_vtbl_env, &PL_vtbl_env, MGVTBL); - StructCopy(&MP_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg) +{ + MAGIC *nmg; + MP_TRACE_e(MP_FUNC, "localizing %%ENV"); + sv_magic(nsv, mg->mg_obj, + mg->mg_type, + NULL, 0); + nmg = mg_find(nsv, mg->mg_type); + nmg->mg_ptr = mg->mg_ptr; + if (mg->mg_virtual == &MP_vtbl_env) { + nmg->mg_virtual = &MP_vtbl_env; + nmg->mg_flags |= MGf_COPY; + nmg->mg_flags |= MGf_LOCAL; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found to localize"); + } + return 1; } -void modperl_env_unload(void) +void modperl_env_init(pTHX) { - /* restore originals */ - StructCopy(&MP_PERL_vtbl_env, &PL_vtbl_env, MGVTBL); - StructCopy(&MP_PERL_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); + /* Remove existing 'E' magic from %ENV */ + /* TODO: Should check there is not multiple 'E' magic! */ + if (!my_perl) + return; + if (!PL_envgv) + return; + if (!SvRMAGICAL(ENVHV)) + return; + if (!mg_find((const SV *)ENVHV, PERL_MAGIC_env)) + return; + mg_free_type((SV*)ENVHV, PERL_MAGIC_env); + + /* Add our version instead */ + sv_magicext((SV*)ENVHV, (SV*)NULL, PERL_MAGIC_env, &MP_vtbl_env, (char*)NULL, 0); } +void modperl_env_unload(pTHX) +{ + /* Remove our 'E' magic from %ENV */ + /* TODO: Should check there is not multiple 'E' magic! */ + if (!my_perl) + return; + if (!PL_envgv) + return; + if (!SvRMAGICAL(ENVHV)) + return; + if (!mg_find((const SV *)ENVHV, PERL_MAGIC_env)) + return; + mg_free_type((SV*)ENVHV, PERL_MAGIC_env); + + /* Restore original */ + sv_magicext((SV*)ENVHV, (SV*)NULL, PERL_MAGIC_env, &PL_vtbl_env, (char*)NULL, 0); +} + /* * Local Variables: * c-basic-offset: 4 Index: src/modules/perl/modperl_env.h =================================================================== --- src/modules/perl/modperl_env.h (revision 1711313) +++ src/modules/perl/modperl_env.h (working copy) @@ -58,9 +58,9 @@ void modperl_env_request_untie(pTHX_ request_rec *r); -void modperl_env_init(void); +void modperl_env_init(pTHX); -void modperl_env_unload(void); +void modperl_env_unload(pTHX); #endif /* MODPERL_ENV_H */ Index: src/modules/perl/modperl_perl.c =================================================================== --- src/modules/perl/modperl_perl.c (revision 1711313) +++ src/modules/perl/modperl_perl.c (working copy) @@ -181,6 +181,8 @@ } } + modperl_env_unload(perl); + perl_destruct(perl); /* XXX: big bug in 5.6.1 fixed in 5.7.2+
From: ntyni [...] iki.fi
Download (untitled) / with headers
text/plain 693b
On Wed Oct 28 18:49:31 2015, ntyni@iki.fi wrote: Show quoted text
> - running t/modperl/env.t and t/modules/cgi.t in the same run makes > the latter fail because $ENV{MOD_PERL} is not set
I seem to have found the cause for this: existing %ENV elements needed their envelem vtable fixed too, otherwise modifying them wouldn't reflect in subprocess_env. I'm attaching an updated patch. This passes all tests for me on both 5.20 and 5.22, but I'm sure there are still wrinkles to iron out. Anyway, it's progress. Steve: in particular, I'm not seeing the setupenv2.t failures you mentioned. Do they still fail with this? See the commit message for remaining concerns. Eyeballs would be very welcome. -- Niko
Subject: 0001-First-steps-at-Perl-5.22-compatibility-take-2.patch
From 4fc74e1abe3c3000d39e0fd2efbc9c9dce4f8fd9 Mon Sep 17 00:00:00 2001 From: Niko Tyni <ntyni@debian.org> Date: Thu, 29 Oct 2015 00:18:53 +0200 Subject: [PATCH] First steps at Perl 5.22 compatibility, take 2 As outlined by Leon Timmermans in [perl #123687]: lookup %ENV's magic, and then replace the pointer to PL_vtbl_env with a pointer to MP_vtbl_env. You may have to add some svt_copy magic to make it cast MP_vtbl_envelem instead of PL_vtbl_envelem on the elements. with an added svt_local for the 'local %ENV' tests. While at it, augment t/modperl/env.t to check that deleting %ENV elements really removes them from subprocess_env. This highlights the need for modifying their vtable, currently with modperl_env_fix_envelem_vtable(). Main open questions: - where should modperl_env_init2() be really called? It needs an initialized Perl interpreter for ENVHV (%ENV), so modperl_sys_init() seems too early - cleanup (replacement for modperl_env_unload()) is nonexistent - modperl_env_fix_envelem_vtable() should probably be merged into modperl_envelem_tie() somehow Bug: https://rt.cpan.org/Public/Bug/Display.html?id=101962 Bug: https://rt.perl.org/Ticket/Display.html?id=123687 --- src/modules/perl/modperl_env.c | 71 ++++++++++++++++++++++++++++++++++ src/modules/perl/modperl_env.h | 4 ++ src/modules/perl/modperl_perl_global.c | 1 + t/response/TestModperl/env.pm | 4 +- 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/modules/perl/modperl_env.c b/src/modules/perl/modperl_env.c index c1a276b..aa27299 100644 --- a/src/modules/perl/modperl_env.c +++ b/src/modules/perl/modperl_env.c @@ -34,6 +34,8 @@ static unsigned long modperl_interp_address(pTHX) #endif } +void modperl_env_fix_envelem_vtable(pTHX_ SV *); + #define MP_ENV_HV_STORE(hv, key, val) STMT_START { \ I32 klen = strlen(key); \ SV **svp = hv_fetch(hv, key, klen, FALSE); \ @@ -46,6 +48,7 @@ static unsigned long modperl_interp_address(pTHX) sv = newSVpv(val, 0); \ (void)hv_store(hv, key, klen, sv, FALSE); \ modperl_envelem_tie(sv, key, klen); \ + modperl_env_fix_envelem_vtable(aTHX_ sv); \ svp = &sv; \ } \ MP_TRACE_e(MP_FUNC, "$ENV{%s} = \"%s\";", key, val); \ @@ -121,6 +124,7 @@ static void modperl_env_table_populate(pTHX_ apr_table_t *table) const apr_array_header_t *array; apr_table_entry_t *elts; + modperl_env_init2(aTHX); modperl_env_untie(mg_flags); array = apr_table_elts(table); @@ -340,6 +344,7 @@ void modperl_env_default_populate(pTHX) sv, ent->hash); MP_TRACE_e(MP_FUNC, "$ENV{%s} = \"%s\";", ent->key, ent->val); modperl_envelem_tie(sv, ent->key, ent->klen); + modperl_env_fix_envelem_vtable(aTHX_ sv); ent++; } @@ -612,6 +617,9 @@ static int modperl_env_magic_get(pTHX_ SV *sv, MAGIC *mg) } #endif +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen); +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg); + /* override %ENV virtual tables with our own */ static MGVTBL MP_vtbl_env = { 0, @@ -619,6 +627,7 @@ static MGVTBL MP_vtbl_env = { 0, modperl_env_magic_clear_all, 0 + , modperl_env_magic_copy, 0, modperl_env_magic_local_all }; static MGVTBL MP_vtbl_envelem = { @@ -629,22 +638,84 @@ static MGVTBL MP_vtbl_envelem = { 0 }; +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen) +{ + MAGIC *nmg = NULL; + sv_magic(nsv, mg->mg_obj, + toLOWER(mg->mg_type), + name, namlen); + nmg = mg_find(nsv, toLOWER(mg->mg_type)); + if (mg->mg_virtual == &MP_vtbl_env) { + MP_TRACE_d(MP_FUNC, "copying magic to %%ENV element"); + nmg->mg_virtual = &MP_vtbl_envelem; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found to copy"); + } + return 1; +} + +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg) +{ + MAGIC *nmg; + MP_TRACE_e(MP_FUNC, "localizing %%ENV"); + sv_magic(nsv, mg->mg_obj, + mg->mg_type, + NULL, 0); + nmg = mg_find(nsv, mg->mg_type); + nmg->mg_ptr = mg->mg_ptr; + if (mg->mg_virtual == &MP_vtbl_env) { + nmg->mg_virtual = &MP_vtbl_env; + nmg->mg_flags |= MGf_COPY; + nmg->mg_flags |= MGf_LOCAL; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found to localize"); + } + return 1; +} + void modperl_env_init(void) { /* save originals */ StructCopy(&PL_vtbl_env, &MP_PERL_vtbl_env, MGVTBL); StructCopy(&PL_vtbl_envelem, &MP_PERL_vtbl_envelem, MGVTBL); +#if 0 /* replace with our versions */ StructCopy(&MP_vtbl_env, &PL_vtbl_env, MGVTBL); StructCopy(&MP_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); +#endif +} + +void modperl_env_init2(pTHX) +{ + MAGIC *mg; + + MP_TRACE_d(MP_FUNC, "changing vtable for %%ENV"); + mg = mg_find((SV *)ENVHV, PERL_MAGIC_env); + if (mg) { + mg->mg_virtual = &MP_vtbl_env; + mg->mg_flags |= MGf_COPY; + mg->mg_flags |= MGf_LOCAL; + } else { + MP_TRACE_d(MP_FUNC, "no %%ENV magic found?"); + } +} + +void modperl_env_fix_envelem_vtable(pTHX_ SV *sv) +{ + MAGIC *mg = mg_find(sv, PERL_MAGIC_envelem); + if (mg) { + mg->mg_virtual = &MP_vtbl_envelem; + } } void modperl_env_unload(void) { +#if 0 /* restore originals */ StructCopy(&MP_PERL_vtbl_env, &PL_vtbl_env, MGVTBL); StructCopy(&MP_PERL_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); +#endif } /* diff --git a/src/modules/perl/modperl_env.h b/src/modules/perl/modperl_env.h index 406e345..9657331 100644 --- a/src/modules/perl/modperl_env.h +++ b/src/modules/perl/modperl_env.h @@ -60,6 +60,10 @@ void modperl_env_request_untie(pTHX_ request_rec *r); void modperl_env_init(void); +void modperl_env_init2(pTHX); + +void modperl_env_fix_envelem_vtable(pTHX_ SV *sv); + void modperl_env_unload(void); #endif /* MODPERL_ENV_H */ diff --git a/src/modules/perl/modperl_perl_global.c b/src/modules/perl/modperl_perl_global.c index f52e42c..99565f8 100644 --- a/src/modules/perl/modperl_perl_global.c +++ b/src/modules/perl/modperl_perl_global.c @@ -272,6 +272,7 @@ static HV *copyENV(pTHX_ HV *ohv) while ((entry = hv_iternext(ohv))) { SV *sv = newSVsv(HeVAL(entry)); modperl_envelem_tie(sv, HeKEY(entry), HeKLEN(entry)); + modperl_env_fix_envelem_vtable(aTHX_ sv); (void)hv_store(hv, HeKEY(entry), HeKLEN(entry), sv, HeHASH(entry)); } diff --git a/t/response/TestModperl/env.pm b/t/response/TestModperl/env.pm index 62eac81..ac057a9 100644 --- a/t/response/TestModperl/env.pm +++ b/t/response/TestModperl/env.pm @@ -15,7 +15,7 @@ use Apache2::Const -compile => 'OK'; sub handler { my $r = shift; - plan $r, tests => 23 + keys(%ENV); + plan $r, tests => 23 + 3 * keys(%ENV); my $env = $r->subprocess_env; @@ -75,6 +75,8 @@ sub handler { for my $key (sort keys %ENV) { eval { delete $ENV{$key}; }; ok t_cmp($@, '', $key); + ok t_cmp($ENV{$key}, undef, "ENV{$key} is empty"); + ok t_cmp($env->get($key), undef, "subprocess_env($key) is empty"); } Apache2::Const::OK; -- 2.6.2
From: ntyni [...] iki.fi
Download (untitled) / with headers
text/plain 534b
On Sun Nov 22 17:50:22 2015, ntyni@iki.fi wrote: Show quoted text
> I'm attaching an updated patch. This passes all tests for me on both > 5.20 and 5.22, but I'm sure there are still wrinkles to iron out.
Here's another version, based on Steve's env2.patch and using sv_magicext() where applicable to get our own vtables in. It's somewhat cleaner and still passes all the tests for me. I've merged the envelem vtable fixup into the modperl_envelem_tie macro, but had to make MP_vtbl_envelem global for that. I'm sure there's a better way. -- Niko
Subject: 0001-Steps-at-Perl-5.22-compatibility-take-3.patch
From 74bd6a91aa7b8ca47d826a1655d06cf295c1365c Mon Sep 17 00:00:00 2001 From: Niko Tyni <ntyni@debian.org> Date: Wed, 4 Nov 2015 21:51:40 +0200 Subject: [PATCH] Steps at Perl 5.22 compatibility, take 3 As outlined by Leon Timmermans in [perl #123687]: lookup %ENV's magic, and then replace the pointer to PL_vtbl_env with a pointer to MP_vtbl_env. You may have to add some svt_copy magic to make it cast MP_vtbl_envelem instead of PL_vtbl_envelem on the elements. with an added svt_local for the 'local %ENV' tests. While at it, augment t/modperl/env.t to check that deleting %ENV elements really removes them from subprocess_env. This highlights the need for modifying their vtable, currently in the modperl_envelem_tie() macro. Main open questions: - MP_vtbl_envelem probably shouldn't be a global variable, but the modperl_envelem_tie() macro needs it. Based on Steve Hay's env2.patch in [rt.cpan.org #101962]. Bug: https://rt.cpan.org/Public/Bug/Display.html?id=101962 Bug: https://rt.perl.org/Ticket/Display.html?id=123687 --- src/modules/perl/mod_perl.c | 7 +--- src/modules/perl/modperl_env.c | 88 +++++++++++++++++++++++++++++++++++------ src/modules/perl/modperl_env.h | 8 ++-- src/modules/perl/modperl_perl.c | 2 + t/response/TestModperl/env.pm | 4 +- 5 files changed, 87 insertions(+), 22 deletions(-) diff --git a/src/modules/perl/mod_perl.c b/src/modules/perl/mod_perl.c index 1148bc0..1e17747 100644 --- a/src/modules/perl/mod_perl.c +++ b/src/modules/perl/mod_perl.c @@ -262,6 +262,8 @@ PerlInterpreter *modperl_startup(server_rec *s, apr_pool_t *p) exit(1); } + modperl_env_init(aTHX); + /* suspend END blocks to be run at server shutdown */ endav = PL_endav; PL_endav = (AV *)NULL; @@ -576,9 +578,6 @@ static apr_status_t modperl_sys_init(void) /* modifies PL_ppaddr */ modperl_perl_pp_set_all(); - /* modifies PL_vtbl_env{elem} */ - modperl_env_init(); - return APR_SUCCESS; } @@ -597,8 +596,6 @@ static apr_status_t modperl_sys_term(void *data) MP_TRACE_i(MP_FUNC, "mod_perl sys term"); - modperl_env_unload(); - modperl_perl_pp_unset_all(); PERL_SYS_TERM(); diff --git a/src/modules/perl/modperl_env.c b/src/modules/perl/modperl_env.c index c1a276b..1aaf3fc 100644 --- a/src/modules/perl/modperl_env.c +++ b/src/modules/perl/modperl_env.c @@ -121,6 +121,7 @@ static void modperl_env_table_populate(pTHX_ apr_table_t *table) const apr_array_header_t *array; apr_table_entry_t *elts; + modperl_env_init(aTHX); modperl_env_untie(mg_flags); array = apr_table_elts(table); @@ -612,16 +613,22 @@ static int modperl_env_magic_get(pTHX_ SV *sv, MAGIC *mg) } #endif +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen); +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg); + /* override %ENV virtual tables with our own */ static MGVTBL MP_vtbl_env = { 0, modperl_env_magic_set_all, 0, modperl_env_magic_clear_all, - 0 + 0, + modperl_env_magic_copy, + 0, + modperl_env_magic_local_all }; -static MGVTBL MP_vtbl_envelem = { +MGVTBL MP_vtbl_envelem = { 0, modperl_env_magic_set, 0, @@ -629,22 +636,77 @@ static MGVTBL MP_vtbl_envelem = { 0 }; -void modperl_env_init(void) +static int modperl_env_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv, const char *name, I32 namlen) { - /* save originals */ - StructCopy(&PL_vtbl_env, &MP_PERL_vtbl_env, MGVTBL); - StructCopy(&PL_vtbl_envelem, &MP_PERL_vtbl_envelem, MGVTBL); + MP_TRACE_e(MP_FUNC, "setting up %%ENV element magic"); + sv_magicext(nsv, mg->mg_obj, + toLOWER(mg->mg_type), + &MP_vtbl_envelem, + name, namlen); - /* replace with our versions */ - StructCopy(&MP_vtbl_env, &PL_vtbl_env, MGVTBL); - StructCopy(&MP_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); + return 1; } -void modperl_env_unload(void) +static int modperl_env_magic_local_all(pTHX_ SV *nsv, MAGIC *mg) { - /* restore originals */ - StructCopy(&MP_PERL_vtbl_env, &PL_vtbl_env, MGVTBL); - StructCopy(&MP_PERL_vtbl_envelem, &PL_vtbl_envelem, MGVTBL); + MAGIC *nmg; + MP_TRACE_e(MP_FUNC, "localizing %%ENV"); + nmg = sv_magicext(nsv, mg->mg_obj, + mg->mg_type, + &MP_vtbl_env, + NULL, 0); + nmg->mg_ptr = mg->mg_ptr; + nmg->mg_flags |= MGf_COPY; + nmg->mg_flags |= MGf_LOCAL; + + return 1; +} + +void modperl_env_init(pTHX) +{ + /* save originals */ + StructCopy(&PL_vtbl_env, &MP_PERL_vtbl_env, MGVTBL); + StructCopy(&PL_vtbl_envelem, &MP_PERL_vtbl_envelem, MGVTBL); + + MAGIC *mg; + /* Remove existing 'E' magic from %ENV */ + /* TODO: Should check there is not multiple 'E' magic! */ + if (!my_perl) + return; + if (!PL_envgv) + return; + if (!SvRMAGICAL(ENVHV)) + return; + mg = mg_find((const SV *)ENVHV, PERL_MAGIC_env); + if (!mg) + return; + if (mg->mg_virtual == &MP_vtbl_env) + return; + MP_TRACE_d(MP_FUNC, "ptr: %x obj: %x flags:%x", mg->mg_ptr, mg->mg_obj, mg->mg_flags); + mg_free_type((SV*)ENVHV, PERL_MAGIC_env); + + /* Add our version instead */ + mg = sv_magicext((SV*)ENVHV, (SV*)NULL, PERL_MAGIC_env, &MP_vtbl_env, (char*)NULL, 0); + mg->mg_flags |= MGf_COPY; + mg->mg_flags |= MGf_LOCAL; +} + +void modperl_env_unload(pTHX) +{ + /* Remove our 'E' magic from %ENV */ + /* TODO: Should check there is not multiple 'E' magic! */ + if (!my_perl) + return; + if (!PL_envgv) + return; + if (!SvRMAGICAL(ENVHV)) + return; + if (!mg_find((const SV *)ENVHV, PERL_MAGIC_env)) + return; + mg_free_type((SV*)ENVHV, PERL_MAGIC_env); + + /* Restore original */ + sv_magicext((SV*)ENVHV, (SV*)NULL, PERL_MAGIC_env, &PL_vtbl_env, (char*)NULL, 0); } /* diff --git a/src/modules/perl/modperl_env.h b/src/modules/perl/modperl_env.h index 406e345..05d6bbd 100644 --- a/src/modules/perl/modperl_env.h +++ b/src/modules/perl/modperl_env.h @@ -28,7 +28,7 @@ MP_magical_tie(ENVHV, mg_flags) #define modperl_envelem_tie(sv, key, klen) \ - sv_magic(sv, (SV *)NULL, 'e', key, klen) + sv_magicext(sv, (SV *)NULL, PERL_MAGIC_envelem, &MP_vtbl_envelem, key, klen) void modperl_env_hash_keys(pTHX); @@ -58,9 +58,11 @@ void modperl_env_request_tie(pTHX_ request_rec *r); void modperl_env_request_untie(pTHX_ request_rec *r); -void modperl_env_init(void); +void modperl_env_init(pTHX); -void modperl_env_unload(void); +void modperl_env_unload(pTHX); + +MGVTBL MP_vtbl_envelem; #endif /* MODPERL_ENV_H */ diff --git a/src/modules/perl/modperl_perl.c b/src/modules/perl/modperl_perl.c index dfe4d36..bda988c 100644 --- a/src/modules/perl/modperl_perl.c +++ b/src/modules/perl/modperl_perl.c @@ -181,6 +181,8 @@ void modperl_perl_destruct(PerlInterpreter *perl) } } + modperl_env_unload(perl); + perl_destruct(perl); /* XXX: big bug in 5.6.1 fixed in 5.7.2+ diff --git a/t/response/TestModperl/env.pm b/t/response/TestModperl/env.pm index 62eac81..ac057a9 100644 --- a/t/response/TestModperl/env.pm +++ b/t/response/TestModperl/env.pm @@ -15,7 +15,7 @@ use Apache2::Const -compile => 'OK'; sub handler { my $r = shift; - plan $r, tests => 23 + keys(%ENV); + plan $r, tests => 23 + 3 * keys(%ENV); my $env = $r->subprocess_env; @@ -75,6 +75,8 @@ sub handler { for my $key (sort keys %ENV) { eval { delete $ENV{$key}; }; ok t_cmp($@, '', $key); + ok t_cmp($ENV{$key}, undef, "ENV{$key} is empty"); + ok t_cmp($env->get($key), undef, "subprocess_env($key) is empty"); } Apache2::Const::OK; -- 2.6.2
CC: Steve Hay <shay [...] cpan.org>, PLICEASE [...] cpan.org, abe+cpan [...] deuxchevaux.org, dam [...] cpan.org, dom [...] cpan.org, ether [...] cpan.org, gregoa [...] cpan.org
Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Tue, 24 Nov 2015 13:16:16 +0000
To: bug-mod_perl [...] rt.cpan.org
From: Steve Hay <steve.m.hay [...] googlemail.com>
On 22 November 2015 at 22:50, ntyni@iki.fi via RT <bug-mod_perl@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > On Wed Oct 28 18:49:31 2015, ntyni@iki.fi wrote: >
>> - running t/modperl/env.t and t/modules/cgi.t in the same run makes >> the latter fail because $ENV{MOD_PERL} is not set
> > I seem to have found the cause for this: existing %ENV elements needed their envelem vtable fixed too, otherwise modifying them wouldn't reflect in subprocess_env. > > I'm attaching an updated patch. This passes all tests for me on both 5.20 and 5.22, but I'm sure there are still wrinkles to iron out. Anyway, it's progress. > > Steve: in particular, I'm not seeing the setupenv2.t failures you mentioned. Do they still fail with this? > > See the commit message for remaining concerns. Eyeballs would be very welcome. >
Sorry for being slow responding to this. This is great news! -- With this patch I have exactly the same set of failures as without the patch using perl-5.20.x (which I think are all Windows-specific).
CC: Steve Hay <shay [...] cpan.org>, PLICEASE [...] cpan.org, abe+cpan [...] deuxchevaux.org, dam [...] cpan.org, dom [...] cpan.org, ether [...] cpan.org, gregoa [...] cpan.org
Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Tue, 24 Nov 2015 13:21:00 +0000
To: bug-mod_perl [...] rt.cpan.org
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 998b
On 23 November 2015 at 21:06, ntyni@iki.fi via RT <bug-mod_perl@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > On Sun Nov 22 17:50:22 2015, ntyni@iki.fi wrote: >
>> I'm attaching an updated patch. This passes all tests for me on both >> 5.20 and 5.22, but I'm sure there are still wrinkles to iron out.
> > Here's another version, based on Steve's env2.patch and using sv_magicext() where applicable to get our own vtables in. It's somewhat cleaner and still passes all the tests for me. > > I've merged the envelem vtable fixup into the modperl_envelem_tie macro, but had to make MP_vtbl_envelem global for that. I'm sure there's a better way.
I liked this version better, despite the MP_vtbl_envelem being global (the Perl equivalent is global anyway, of course), but unfortunately it's not working for me: I deleted two errant StructCopy() calls from the start of modperl_env_init() which were accidentally left behind, but httpd is crashing on startup :-(
From: ntyni [...] iki.fi
Download (untitled) / with headers
text/plain 610b
On Tue Nov 24 08:21:14 2015, steve.m.hay@googlemail.com wrote: Show quoted text
> I liked this version better, despite the MP_vtbl_envelem being global > (the Perl equivalent is global anyway, of course), but unfortunately > it's not working for me: I deleted two errant StructCopy() calls from > the start of modperl_env_init() which were accidentally left behind, > but httpd is crashing on startup :-(
Hm, I think I restored MP_PERL_vtbl_env and MP_PERL_vtbl_envelem initialization on purpose, as they are used by the MP_PL_vtbl_call() invocations. But I guess you had problems already before re-deleting those? -- Niko
Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Tue, 24 Nov 2015 13:41:18 +0000
To: bug-mod_perl [...] rt.cpan.org
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 1018b
On 24 November 2015 at 13:38, ntyni@iki.fi via RT <bug-mod_perl@rt.cpan.org> wrote: Show quoted text
> Queue: mod_perl > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > On Tue Nov 24 08:21:14 2015, steve.m.hay@googlemail.com wrote: >
>> I liked this version better, despite the MP_vtbl_envelem being global >> (the Perl equivalent is global anyway, of course), but unfortunately >> it's not working for me: I deleted two errant StructCopy() calls from >> the start of modperl_env_init() which were accidentally left behind, >> but httpd is crashing on startup :-(
> > Hm, I think I restored MP_PERL_vtbl_env and MP_PERL_vtbl_envelem initialization on purpose, as they are used by the MP_PL_vtbl_call() invocations. > > But I guess you had problems already before re-deleting those? >
Ah! I hadn't realized that. I will put them back and fix the problem that I had by other means -- it was simply that the build failed due to mg being declared after them. (VC++ doesn't allow mixed code and declarations.)
Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Tue, 24 Nov 2015 13:51:03 +0000
To: bug-mod_perl [...] rt.cpan.org
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 1.3k
On 24 November 2015 at 13:41, Steve Hay <steve.m.hay@googlemail.com> wrote: Show quoted text
> On 24 November 2015 at 13:38, ntyni@iki.fi via RT > <bug-mod_perl@rt.cpan.org> wrote:
>> Queue: mod_perl >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > >> >> On Tue Nov 24 08:21:14 2015, steve.m.hay@googlemail.com wrote: >>
>>> I liked this version better, despite the MP_vtbl_envelem being global >>> (the Perl equivalent is global anyway, of course), but unfortunately >>> it's not working for me: I deleted two errant StructCopy() calls from >>> the start of modperl_env_init() which were accidentally left behind, >>> but httpd is crashing on startup :-(
>> >> Hm, I think I restored MP_PERL_vtbl_env and MP_PERL_vtbl_envelem initialization on purpose, as they are used by the MP_PL_vtbl_call() invocations. >> >> But I guess you had problems already before re-deleting those? >>
> > Ah! I hadn't realized that. I will put them back and fix the problem > that I had by other means -- it was simply that the build failed due > to mg being declared after them. (VC++ doesn't allow mixed code and > declarations.)
Although we still shouldn't be needing to make copies of the original Perl vtables now that we aren't changing them anyway! MP_PL_vtbl_call should surely just be using PL_vtbl_env* directly instead of MP_PERL_vtbl_env*?
From: ntyni [...] iki.fi
Download (untitled) / with headers
text/plain 343b
On Tue Nov 24 08:51:18 2015, steve.m.hay@googlemail.com wrote: Show quoted text
> Although we still shouldn't be needing to make copies of the original > Perl vtables now that we aren't changing them anyway! MP_PL_vtbl_call > should surely just be using PL_vtbl_env* directly instead of > MP_PERL_vtbl_env*?
Heh, good point. I'm sure you're right :) -- Niko
Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Tue, 24 Nov 2015 14:11:29 +0000
To: bug-mod_perl [...] rt.cpan.org
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 774b
On 24 November 2015 at 13:58, ntyni@iki.fi via RT <bug-mod_perl@rt.cpan.org> wrote: Show quoted text
> Queue: mod_perl > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > On Tue Nov 24 08:51:18 2015, steve.m.hay@googlemail.com wrote: >
>> Although we still shouldn't be needing to make copies of the original >> Perl vtables now that we aren't changing them anyway! MP_PL_vtbl_call >> should surely just be using PL_vtbl_env* directly instead of >> MP_PERL_vtbl_env*?
> > Heh, good point. I'm sure you're right :)
All tests (bar the sadly now usual Windows failures) successful with the attached slight variation of your take 3 patch. I will look to tidy this up and get it committed and then get the ball rolling on 2.0.10 :-) Thank you for your work on this!

Message body is not shown because sender requested not to inline it.

Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Tue, 24 Nov 2015 12:40:00 -0500
To: "bug-mod_perl [...] rt.cpan.org" <bug-mod_perl [...] rt.cpan.org>
From: "Philip M. Gollucci" <pgollucci [...] p6m7g8.com>
Download (untitled) / with headers
text/plain 1.6k
Thanks guys. I agree. Wish I had more time! On Tuesday, November 24, 2015, steve.m.hay@googlemail.com via RT < bug-mod_perl@rt.cpan.org> wrote: Show quoted text
> Queue: mod_perl > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > On 24 November 2015 at 13:58, ntyni@iki.fi <javascript:;> via RT > <bug-mod_perl@rt.cpan.org <javascript:;>> wrote:
> > Queue: mod_perl > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > > > On Tue Nov 24 08:51:18 2015, steve.m.hay@googlemail.com <javascript:;>
> wrote:
> >
> >> Although we still shouldn't be needing to make copies of the original > >> Perl vtables now that we aren't changing them anyway! MP_PL_vtbl_call > >> should surely just be using PL_vtbl_env* directly instead of > >> MP_PERL_vtbl_env*?
> > > > Heh, good point. I'm sure you're right :)
> > All tests (bar the sadly now usual Windows failures) successful with > the attached slight variation of your take 3 patch. > > I will look to tidy this up and get it committed and then get the ball > rolling on 2.0.10 :-) > > Thank you for your work on this! > >
-- --------------------------------------------------------------------------------- Curb: Your ride is here 4096R/D21D2752 <http://pgp.mit.edu/pks/lookup?op=get&search=0xF699A450D21D2752> ECDF B597 B54B 7F92 753E E0EA F699 A450 D21D 2752 Philip M. Gollucci (pgollucci@p6m7g8.com) c: 703.336.9354 Member, Apache Software Foundation Committer, FreeBSD Foundation Consultant, P6M7G8 Inc. Sr. Director IT Operations, Curb What doesn't kill us can only make us stronger; Except it almost kills you.
Download (untitled) / with headers
text/plain 1.4k
On Tue Nov 24 09:11:41 2015, steve.m.hay@googlemail.com wrote: Show quoted text
> On 24 November 2015 at 13:58, ntyni@iki.fi via RT > <bug-mod_perl@rt.cpan.org> wrote:
> > Queue: mod_perl > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > > > On Tue Nov 24 08:51:18 2015, steve.m.hay@googlemail.com wrote: > >
> >> Although we still shouldn't be needing to make copies of the original > >> Perl vtables now that we aren't changing them anyway! MP_PL_vtbl_call > >> should surely just be using PL_vtbl_env* directly instead of > >> MP_PERL_vtbl_env*?
> > > > Heh, good point. I'm sure you're right :)
> > All tests (bar the sadly now usual Windows failures) successful with > the attached slight variation of your take 3 patch. > > I will look to tidy this up and get it committed and then get the ball > rolling on 2.0.10 :-) > > Thank you for your work on this!
Now committed with minor tweaks in rev. 1717474. One point that I wasn't entirely clear about: Why is the modperl_env_init() call required before modperl_env_untie() in modperl_env_table_populate()? Things break without it, but I'm not totally clear about what's happening there. There are other calls to modperl_env_untie() too... Do any of them similarly need a modperl_env_init() call beforehand? I can see how modperl_env_clear() and modperl_env_table_unpopulate() are rather different, but I do wonder if modperl_env_default_populate() needs a modperl_env_init() call too?
From: ntyni [...] iki.fi
Download (untitled) / with headers
text/plain 889b
On Tue Dec 01 12:46:46 2015, SHAY wrote: Show quoted text
> Now committed with minor tweaks in rev. 1717474.
Thanks! Show quoted text
> One point that I wasn't entirely clear about: Why is the > modperl_env_init() call required before modperl_env_untie() in > modperl_env_table_populate()? Things break without it, but I'm not > totally clear about what's happening there. There are other calls to > modperl_env_untie() too... Do any of them similarly need a > modperl_env_init() call beforehand? I can see how modperl_env_clear() > and modperl_env_table_unpopulate() are rather different, but I do > wonder if modperl_env_default_populate() needs a modperl_env_init() > call too?
I think I just put it there because it made the test suite crashes go away :) I agree that part needs attention. Looks like moving it to modperl_env_request_populate() works just as well wrt. the test suite, but that's as far as I got.
Download (untitled) / with headers
text/plain 1.6k
Le Mar 01 Déc 2015 18:46:46, SHAY a écrit : Show quoted text
> On Tue Nov 24 09:11:41 2015, steve.m.hay@googlemail.com wrote:
> > On 24 November 2015 at 13:58, ntyni@iki.fi via RT > > <bug-mod_perl@rt.cpan.org> wrote:
> > > Queue: mod_perl > > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > > > > > On Tue Nov 24 08:51:18 2015, steve.m.hay@googlemail.com wrote: > > >
> > >> Although we still shouldn't be needing to make copies of the > > >> original > > >> Perl vtables now that we aren't changing them anyway! > > >> MP_PL_vtbl_call > > >> should surely just be using PL_vtbl_env* directly instead of > > >> MP_PERL_vtbl_env*?
> > > > > > Heh, good point. I'm sure you're right :)
> > > > All tests (bar the sadly now usual Windows failures) successful with > > the attached slight variation of your take 3 patch. > > > > I will look to tidy this up and get it committed and then get the > > ball > > rolling on 2.0.10 :-) > > > > Thank you for your work on this!
> > > Now committed with minor tweaks in rev. 1717474. > > One point that I wasn't entirely clear about: Why is the > modperl_env_init() call required before modperl_env_untie() in > modperl_env_table_populate()? Things break without it, but I'm not > totally clear about what's happening there. There are other calls to > modperl_env_untie() too... Do any of them similarly need a > modperl_env_init() call beforehand? I can see how modperl_env_clear() > and modperl_env_table_unpopulate() are rather different, but I do > wonder if modperl_env_default_populate() needs a modperl_env_init() > call too?
Has there been some headway made in this regard ? Perl 5.24 is only a couple of months away...
Subject: Re: [rt.cpan.org #101962] modperl_env_init() needs changing for perl >= 5.21.8
Date: Wed, 2 Mar 2016 18:57:02 -0500
To: "bug-mod_perl [...] rt.cpan.org" <bug-mod_perl [...] rt.cpan.org>
From: "Philip M. Gollucci" <pgollucci [...] p6m7g8.com>
Download (untitled) / with headers
text/plain 2.5k
Etime. On Wednesday, March 2, 2016, Mathieu Arnold via RT <bug-mod_perl@rt.cpan.org> wrote: Show quoted text
> Queue: mod_perl > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > Le Mar 01 Déc 2015 18:46:46, SHAY a écrit :
> > On Tue Nov 24 09:11:41 2015, steve.m.hay@googlemail.com <javascript:;>
> wrote:
> > > On 24 November 2015 at 13:58, ntyni@iki.fi <javascript:;> via RT > > > <bug-mod_perl@rt.cpan.org <javascript:;>> wrote:
> > > > Queue: mod_perl > > > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=101962 > > > > > > > > > On Tue Nov 24 08:51:18 2015, steve.m.hay@googlemail.com
> <javascript:;> wrote:
> > > >
> > > >> Although we still shouldn't be needing to make copies of the > > > >> original > > > >> Perl vtables now that we aren't changing them anyway! > > > >> MP_PL_vtbl_call > > > >> should surely just be using PL_vtbl_env* directly instead of > > > >> MP_PERL_vtbl_env*?
> > > > > > > > Heh, good point. I'm sure you're right :)
> > > > > > All tests (bar the sadly now usual Windows failures) successful with > > > the attached slight variation of your take 3 patch. > > > > > > I will look to tidy this up and get it committed and then get the > > > ball > > > rolling on 2.0.10 :-) > > > > > > Thank you for your work on this!
> > > > > > Now committed with minor tweaks in rev. 1717474. > > > > One point that I wasn't entirely clear about: Why is the > > modperl_env_init() call required before modperl_env_untie() in > > modperl_env_table_populate()? Things break without it, but I'm not > > totally clear about what's happening there. There are other calls to > > modperl_env_untie() too... Do any of them similarly need a > > modperl_env_init() call beforehand? I can see how modperl_env_clear() > > and modperl_env_table_unpopulate() are rather different, but I do > > wonder if modperl_env_default_populate() needs a modperl_env_init() > > call too?
> > > Has there been some headway made in this regard ? Perl 5.24 is only a > couple of months away... >
-- --------------------------------------------------------------------------------- 4096R/D21D2752 <http://pgp.mit.edu/pks/lookup?op=get&search=0xF699A450D21D2752> ECDF B597 B54B 7F92 753E E0EA F699 A450 D21D 2752 Philip M. Gollucci (pgollucci@p6m7g8.com) c: 703.336.9354 Member, Apache Software Foundation Committer, FreeBSD Foundation Consultant, P6M7G8 Inc. Director Cloud Technology, Capital One What doesn't kill us can only make us stronger; Except it almost kills you.
Download (untitled) / with headers
text/plain 246b
I hear that a patch for this has been in git for a while now. Would it be possible to get a release of this soon? At $work we are blocked from upgrading our perl simply because of mod_perl. Thanks for maintaining this and keeping things running!
Download (untitled) / with headers
text/plain 686b
On Mon Jun 27 10:29:23 2016, EXODIST wrote: Show quoted text
> I hear that a patch for this has been in git for a while now. Would it > be possible to get a release of this soon? At $work we are blocked > from upgrading our perl simply because of mod_perl. > > Thanks for maintaining this and keeping things running!
Yes, it's tentatively fixed in SVN. Ideally it could do with more eyes to verify its correctness, but it's been a while now with no further comments on this ticket and in the meantime I believe Debian (at least) have started shipping the current SVN... I've not (yet) heard of any problems reported there, so maybe it's time to make a new release. I will try to make that happen soon.
Download (untitled) / with headers
text/plain 766b
On Mon Jun 27 09:43:15 2016, SHAY wrote: Show quoted text
> On Mon Jun 27 10:29:23 2016, EXODIST wrote:
> > I hear that a patch for this has been in git for a while now. Would > > it > > be possible to get a release of this soon? At $work we are blocked > > from upgrading our perl simply because of mod_perl. > > > > Thanks for maintaining this and keeping things running!
> > Yes, it's tentatively fixed in SVN. Ideally it could do with more eyes > to verify its correctness, but it's been a while now with no further > comments on this ticket and in the meantime I believe Debian (at > least) have started shipping the current SVN... I've not (yet) heard > of any problems reported there, so maybe it's time to make a new > release. I will try to make that happen soon.
Thanks!
Now fixed in mod_perl-2.0.10.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.