On Tuesday, 14 February 2012 09:58:51 Steve Hay wrote:
> The patch below fixes the problems with MP_LIBNAME (and cwd) not being set
> correctly when building Apache::Reload and Apache::SizeLimit.
>
> The problem was that the file-level $b variable was created only once when
> ModPerl::MM was first loaded, but that happened before
> Apache2::BuildConfig.pm was written. The patch simply rewrites $b (instead
> of writing a lexical $build which was never used!) later on, by which time
> Apache2::BuildConfig.pm has been created.
>
> If the patch looks ok then shall I apply it before RC2 is rolled?
>
> (Needless to say, this doesn't fix the more serious problem of the test
> suite still failing when httpd.exe crashes with an out of memory error
> part-way through...)
>
> Index: lib/ModPerl/MM.pm
> ===================================================================
> --- lib/ModPerl/MM.pm (revision 1240026)
> +++ lib/ModPerl/MM.pm (working copy)
> @@ -128,7 +128,7 @@
> $eu_mm_mv_all_methods_overriden++;
> }
>
> - my $build = build_config();
> + $b = build_config();
> my_import(__PACKAGE__);
>
> # set top-level WriteMakefile's values if weren't set already
Apart from the fact that $b is a really nasty name for a global variable I
don't mind. But that's not your fault. Be it $x or $y or even $B I wouldn't
complain either. But $a and $b look like out of a sort {} block to me.
Why don't you commit the patch and see what happens. The good thing with SVN
is that we can always go back.
On the OOM, is there a special commit where it starts?
While working on the threading branch I discovered at least one occurrence
(modperl_filter_f_cleanup) where an interpreter was used *after* it has been
put back to the pool. That means at this point it might have already been
assigned to another thread. Thanks, Steve, for reminding that Perl sometimes
picks the interpreter by PERL_GET_CONTEXT. That was the key. See "svn log
-c1243509" for more information. I don't know if the bug is also present in
trunk but I strongly believe so. In principle this may cause arbitrary
effects. The other occurrence mentioned in the commit message can happen only
with "PerlInterpScope handler", I think.
Another note, I have seen quite a few pieces of code reading:
if (...) {
/* should not happen */
return NULL;
}
This is really bad because it hides bugs. It does not fix anything. Wouldn't
it be better to abort() here? Or ap_assert()?
Torsten Förtsch
--
Need professional modperl support? Hire me! (http://foertsch.name)
Like fantasy? http://kabatinte.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@perl.apache.org
For additional commands, e-mail: dev-h...@perl.apache.org
