Re: mp1 and %INC (was Re: 5.8.2 perldelta)
Geoffrey Young wrote: Stas Bekman wrote: Here is the problem. If a module fails to load once it must not be blacklisted. The test demonstrates how a module may fail if loaded from the wrong directory, but then is supposed to succeed if moved to a different directory. this is somewhat addressed in the original thread: http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2003-09/msg01486.html http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2003-09/msg01518.html I think Rick makes a good point - in a persistent environment like mod_perl the file contents (or location) ought not to change during production, and for development we have the Apache::Reload/PerlFreshRestart options. and, as it turns out, it's PerlFreshRestart that is the culprit here - it explicitly sets each %INC entry to undef to allow for a reload, which now is a problem, since undef is now the trigger for a bad attempt. [...] however, if perl isn't interested in changing current behavior, here is a patch that compensates for the changes within mod_perl. +1 for this patch. Great job, Geoff! Index: src/modules/perl/perl_util.c === RCS file: /home/cvspublic/modperl/src/modules/perl/perl_util.c,v retrieving revision 1.52 diff -u -r1.52 perl_util.c --- src/modules/perl/perl_util.c14 Mar 2003 05:03:16 - 1.52 +++ src/modules/perl/perl_util.c30 Oct 2003 15:28:24 - @@ -500,10 +500,9 @@ "%s not found in %%INC\n", elts[i].key)); continue; } - SvREFCNT_dec(HeVAL(entry)); - HeVAL(entry) = &sv_undef; - MP_TRACE_g(fprintf(stderr, "reloading %s\n", HeKEY(entry))); - perl_require_pv(HeKEY(entry)); +hv_delete_ent(hash, keysv, G_DISCARD, 0); + MP_TRACE_g(fprintf(stderr, "reloading %s\n", elts[i].key)); + perl_require_pv(elts[i].key); } SvREFCNT_dec(keysv); } -- __ Stas BekmanJAm_pH --> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: mp1 and %INC (was Re: 5.8.2 perldelta)
On Thu, Oct 30, 2003 at 10:30:19AM -0500, Geoffrey Young wrote: > "The following patch arranges for failures to be cached in %INC as undef > values (success is still cached as filename). I think this should be OK; > I'm not aware of any other meaning for undef values in %INC (and neither is > the test suite)." > > I think it would help mod_perl (and possibly others) is the trigger were > something other than undef - a specific marker could blacklist the module, > while undef could continue to work as before and allow for a reload. I believe that PL_sv_placeholder is available for use, and won't show up on any keys %INC However, for maint I'd prefer a patch which doesn't change the semantics of (what is visible by perl in) %INC The current patch is backed out of maint (but still in blead) As maint pumpking I'd prefer not to be the one interacting with future revisions in the blead tree. I believe that there are at least 6 other people on p5p with blead commit. [I'd prefer to stay blinkered and just deal with p4 integrate commands] Nicholas Clark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: 5.8.2-RC1 and mp2
I made some progress on making sure that the problem is reproducable. Rasing
#define HV_MAX_LENGTH_BEFORE_SPLIT 14
makes it a much higher trashold for most normal keys to trigger rehashing, but
I'm happy that the new test is happily failing to find a previously cached
stash, despite the high treshold.
I have two remaining issues on the test side (before trying to fix things):
1) replace the hardcoded attack input (from the original attack report by
Scott) to something that's autogenerated. I want a sub that I can ask for N
number of hash keys that will collapse into the same list.
2) I need some way to verify that the attack has been successfully performed,
so if in the future the hashing algorithm or the threshold change this test
won't be misleadingly successful even though the problem may exist. Since Nick
added a special flag to HVs that were re-hashed, can we get a B:: function
that can check for this flag?
At the moment I do a visual check, by looking for the debug print I've planted
into hv.c, so error_log looks like:
==> starting
We are under attack. But Do Not Panic
==> ending
[Thu Oct 30 16:37:08 2003] [error] lookup of 'TestPerl::hash_attack::handler'
failed
Here is the test:
package TestPerl::hash_attack;
use strict;
use warnings FATAL => 'all';
use Apache::Test;
use Apache::TestUtil;
use Apache::TestTrace;
use Apache::Const -compile => 'OK';
my $input = <
# create conditions which will trigger a rehash on the current stash
# (__PACKAGE)
sub fixup {
my $r = shift;
no strict 'refs';
debug "starting attack";
for (split /\n/, $input) {
my $symbol = __PACKAGE__ . "::$_";
#autovivify
$$symbol++;
}
debug "ending attack";
return Apache::DECLINED;
}
# if the rehashing of the keys in the stash happens due to the hash attack,
# mod_perl must not fail to find the previously cached stash (response
# handler in this case)
sub handler {
my $r = shift;
plan $r, tests => 1;
ok 1;
return Apache::OK;
}
1;
__END__
PerlModule TestPerl::hash_attack
PerlFixupHandler TestPerl::hash_attack::fixup
__
Stas BekmanJAm_pH --> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
mp1 and %INC (was Re: 5.8.2 perldelta)
Stas Bekman wrote: Here is the problem. If a module fails to load once it must not be blacklisted. The test demonstrates how a module may fail if loaded from the wrong directory, but then is supposed to succeed if moved to a different directory. this is somewhat addressed in the original thread: http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2003-09/msg01486.html http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2003-09/msg01518.html I think Rick makes a good point - in a persistent environment like mod_perl the file contents (or location) ought not to change during production, and for development we have the Apache::Reload/PerlFreshRestart options. and, as it turns out, it's PerlFreshRestart that is the culprit here - it explicitly sets each %INC entry to undef to allow for a reload, which now is a problem, since undef is now the trigger for a bad attempt. from rick's patch email: "The following patch arranges for failures to be cached in %INC as undef values (success is still cached as filename). I think this should be OK; I'm not aware of any other meaning for undef values in %INC (and neither is the test suite)." I think it would help mod_perl (and possibly others) is the trigger were something other than undef - a specific marker could blacklist the module, while undef could continue to work as before and allow for a reload. however, if perl isn't interested in changing current behavior, here is a patch that compensates for the changes within mod_perl. --Geoff Index: src/modules/perl/perl_util.c === RCS file: /home/cvspublic/modperl/src/modules/perl/perl_util.c,v retrieving revision 1.52 diff -u -r1.52 perl_util.c --- src/modules/perl/perl_util.c14 Mar 2003 05:03:16 - 1.52 +++ src/modules/perl/perl_util.c30 Oct 2003 15:28:24 - @@ -500,10 +500,9 @@ "%s not found in %%INC\n", elts[i].key)); continue; } - SvREFCNT_dec(HeVAL(entry)); - HeVAL(entry) = &sv_undef; - MP_TRACE_g(fprintf(stderr, "reloading %s\n", HeKEY(entry))); - perl_require_pv(HeKEY(entry)); +hv_delete_ent(hash, keysv, G_DISCARD, 0); + MP_TRACE_g(fprintf(stderr, "reloading %s\n", elts[i].key)); + perl_require_pv(elts[i].key); } SvREFCNT_dec(keysv); } - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
