-
Notifications
You must be signed in to change notification settings - Fork 561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
commit 2e6f1ae9c: breaks blead #15945
Comments
From @khwilliamsonThis is a bug report for perl from khw@cpan.org, commit 2e6f1ae fix and test execution of non-empty .bs files This commit consistently causes this: panic: free from wrong pool, eea800!=d40c20 during global destruction. but only under certain circumstances. One that threw us off scent, is I am using ccache. I have tried clearing it before running, with no You can see the Configure options below. I run on an 8-core system with Flags: Site configuration information for perl 5.26.0: Configured by khw at Fri Apr 7 16:55:52 MDT 2017. Summary of my perl5 (revision 5 version 26 subversion 0) configuration: @INC for perl 5.26.0: Environment for perl 5.26.0: PATH=/usr/lib/ccache:/home/khw/bin:/home/khw/perl5/perlbrew/bin:/home/khw/print/bin:/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/usr/games:/usr/local/games:/home/khw/iands/www:/home/khw/cxoffice/bin |
From @jkeenanOn Sat, 08 Apr 2017 04:43:13 GMT, public@khwilliamson.com wrote:
I have not been able to reproduce this problem. I configured like this: ##### ... then called 'make test_harness' in a terminal where $TEST_JOBS = 8 and I have 4 cores. All tests PASSed. I then changed the name of the directory from 'blead' to 'rando', then reconfigured changing only the last switch. Again, all tests PASSed. I did not use ccache in either case. In situations like this I try to start from as few switches as possible and add switches until the problem occurs. Conversely, you could also start subtracting switches from what you currently have. I know from backscrolling #p5p that you've done some of that. (I should note that normally I configure with just '-des -Dusedevel' with possibly '-Duseithreads'. 'make' runs without warnings for me. But with all your switches, I get warnings like this: ##### End parenthesis.) And, as you note, your @INC looks very strange. Thank you very much. -- |
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Fri, Apr 07, 2017 at 09:43:13PM -0700, karl williamson wrote:
Is it definitely object2.t which is generating the panic? cd dist/threads-shared/ And if so can the test sccipt be reduced, e.g. perhaps just loading an Alternatively, if you reduce your Configure options, does the problem go -- |
From @khwilliamsonOn 04/10/2017 04:55 AM, Dave Mitchell wrote:
Test Summary Report ../dist/threads-shared/t/object2.t (Wstat: I'm now trying your other ideas. |
From @khwilliamsonOn 04/10/2017 04:55 AM, Dave Mitchell wrote:
The problem doesn't arise when running the test in isolation. I already The line of code that changed to cause the problem, is indeed The problem persists when I whittle down the Configure script to /bin/sh ./Configure -des '-Dprefix=/home/khw/tempdevel' -Dusedevel That means it fails for both gcc and g++, and both -O0 and -O2. tempdevel/ is empty. I removed the earlier /home/khw/devel. And, now I tried this on my laptop, which has a later gcc, and only 4 cores, and |
From @iabynOn Mon, Apr 10, 2017 at 12:38:21PM -0600, Karl Williamson wrote:
Does it require running multiple jobs in parallel to fail? So does ./perl harness ../dist/threads-shared/t/object2.t on its own fail? If not, how about running just a minimal number of tests HARNESS_OPTIONS=j8 ./perl harness ../dist/threads-shared/t/object2.t \ If harness fails with just the one test script, then maybe invoking PERL5LIB=../../lib:../../t Other than that I'm totally bemused. -- |
From @khwilliamsonOn 04/10/2017 05:08 PM, Dave Mitchell wrote:
No If not, how about running just a minimal number of tests
I haven't tried this, but I did try a make test.valgrind with 12
|
From @khwilliamsonOn 04/10/2017 09:09 PM, Karl Williamson wrote:
I managed to get this to fail on an individual run, by grepping for all EMXSHELL=sh PERL_DL_NONLAZY=1 ../../perl -I../.. -MTestInit=U2T Removing either of the environment settings causes it to not fail. The If I add PERL5LIB=../../lib:../../t it also succeeds, which makes me |
From @iabynOn Tue, Apr 11, 2017 at 05:01:25PM -0600, Karl Williamson wrote:
The PERL5LIB and PERL_CORE is being set by TestInit itself - I was dumping Having said that, I still can't reproduce it. Can you run it under valgrind? Also, can you run it under strace to see what files (and paths) it's -- |
From @khwilliamsonOn 04/12/2017 01:15 AM, Dave Mitchell wrote:
Both attached. |
From @khwilliamson |
From @iabynOn Wed, Apr 12, 2017 at 09:23:07AM -0600, Karl Williamson wrote:
The strace shows that the only interaction it's having with .bs files is stat("../../lib/auto/Cwd/Cwd.bs", 0x1221eb0) = -1 ENOENT (No such file or directory) which strongly indicates that the modified branch in XSLoader.pm is never if (-s $bs) { # only read file if it's not empty Could you temporarily replace the 'eval' line with a die to confim that If so, it would indicate that my commit only changed some code which is The valgrind stack trace shows that Perl_croak_nocontext("panic: free from wrong pool") is being called when free()ing the main threads's SV malloc()ed arenas - -- |
From @khwilliamsonOn 04/12/2017 09:48 AM, Dave Mitchell wrote:
The die doesn't get called, but the pool bug is gone too. |
From @khwilliamsonOn 04/12/2017 10:37 AM, Karl Williamson wrote:
Just to be sure, I replaced the die by opening a file and writing to it, If I uncomment the line print STDERR "BS: the panic goes away, but, and this may be important, this message gets Use of uninitialized value in subroutine entry at ../../lib/XSLoader.pm I fail to see how this could be connected except through some weirdness. |
From @iabynOn Wed, Apr 12, 2017 at 04:58:43PM -0600, Karl Williamson wrote:
That's just bizarre. Maybe run it under gdb and see where and what in the Dynaloader XS is -- |
From @khwilliamsonOn 04/13/2017 02:04 AM, Dave Mitchell wrote:
Running under valgrind did not show any errors: Here's the gdb stack trace #0 S_find_uninit_var (my_perl=0xad0c20, obase=0xb92ac8, |
From @khwilliamsonOn 04/13/2017 10:05 AM, Karl Williamson wrote:
Some more data points. I tried it compiling under clang, with I then tried uncommenting that print statement that should not be # dlutils.c before 5.006 has this: It still makes no sense to me that this should happen, given that the
|
From @iabynOn Thu, Apr 13, 2017 at 11:12:18AM -0600, Karl Williamson wrote:
It's this code in dl_generic_private_init(): #ifdef DEBUGGING Normally *DynaLoader::dl_debug doesn't exist, so sv is null. By So I think this is a red herring. Perhaps another approach concerning this panic: panic: free from wrong pool, eea800!=d40c20 during global destruction. would be to analyse it under gdb. When PERL_TRACK_MEMPOOL is defined, So run the failing process once in gdb, breakpointing on the panic line at Then do a Full backtracks at those points might illuminate things a bit. -- |
From @khwilliamsonOn 04/13/2017 12:39 PM, Dave Mitchell wrote:
... Hardware watchpoint 2: -location header->interpreter Old value = <unreadable> Thread 1 "perl" hit Hardware watchpoint 2: -location header->interpreter Old value = (PerlInterpreter *) 0xd40ae0 Thread 1 "perl" hit Breakpoint 1, Perl_safesysfree (where=0xd3b3e0) at |
From @iabynOn Thu, Apr 13, 2017 at 09:48:09PM -0600, Karl Williamson wrote:
Aha, that was the vital info I needed. Turns out the @INC changes were a commit defb77b threads::shared: alloc arenas with correct context Affected files ... Differences ... Inline Patchdiff --git a/dist/threads-shared/lib/threads/shared.pm b/dist/threads-shared/lib/threads/shared.pm
index 5a203b0..73c4dd9 100644
--- a/dist/threads-shared/lib/threads/shared.pm
+++ b/dist/threads-shared/lib/threads/shared.pm
@@ -7,7 +7,7 @@ use warnings;
use Scalar::Util qw(reftype refaddr blessed);
-our $VERSION = '1.55'; # Please update the pod, too.
+our $VERSION = '1.56'; # Please update the pod, too.
my $XS_VERSION = $VERSION;
$VERSION = eval $VERSION;
@@ -195,7 +195,7 @@ threads::shared - Perl extension for sharing data structures between threads
=head1 VERSION
-This document describes threads::shared version 1.55
+This document describes threads::shared version 1.56
=head1 SYNOPSIS
diff --git a/dist/threads-shared/shared.xs b/dist/threads-shared/shared.xs
index dab5e36..3c1b5e6 100644
--- a/dist/threads-shared/shared.xs
+++ b/dist/threads-shared/shared.xs
@@ -1104,8 +1104,9 @@ sharedsv_array_mg_CLEAR(pTHX_ SV *sv, MAGIC *mg)
if (!sv) continue;
if ( (SvOBJECT(sv) || (SvROK(sv) && (sv = SvRV(sv))))
&& SvREFCNT(sv) == 1 ) {
- SV *tmp = Perl_sv_newmortal(caller_perl);
+ SV *tmp;
PERL_SET_CONTEXT((aTHX = caller_perl));
+ tmp = sv_newmortal();
sv_upgrade(tmp, SVt_RV);
get_RV(tmp, sv);
PERL_SET_CONTEXT((aTHX = PL_sharedsv_space));
@@ -1384,8 +1385,9 @@ STORESIZE(SV *obj,IV count)
if ( (SvOBJECT(sv) || (SvROK(sv) && (sv = SvRV(sv))))
&& SvREFCNT(sv) == 1 )
{
- SV *tmp = Perl_sv_newmortal(caller_perl);
+ SV *tmp;
PERL_SET_CONTEXT((aTHX = caller_perl));
+ tmp = sv_newmortal();
sv_upgrade(tmp, SVt_RV);
get_RV(tmp, sv);
PERL_SET_CONTEXT((aTHX = PL_sharedsv_space));
diff --git a/dist/threads-shared/t/object2.t b/dist/threads-shared/t/object2.t
index 3d795b9..31c3797 100644
--- a/dist/threads-shared/t/object2.t
+++ b/dist/threads-shared/t/object2.t
@@ -17,7 +17,7 @@ use ExtUtils::testlib;
BEGIN {
$| = 1;
- print("1..131\n"); ### Number of tests that will be run ###
+ print("1..133\n"); ### Number of tests that will be run ###
};
use threads;
@@ -445,6 +445,28 @@ ok($destroyed[$ID], 'Scalar object removed from shared scalar');
::ok($count == $n, "remove array object by undef");
}
+# RT #131124
+# Emptying a shared array creates new temp SVs. If there are no spare
+# SVs, a new arena is allocated. shared.xs was mallocing a new arena
+# with the wrong perl context set, meaning that when the arena was later
+# freed, it would "panic: realloc from wrong pool"
+#
+
+{
+ threads->new(sub {
+ my @a :shared;
+ push @a, bless &threads::shared::share({}) for 1..1000;
+ undef @a; # this creates lots of temp SVs
+ })->join;
+ ok(1, "#131124 undef array doesnt panic");
+
+ threads->new(sub {
+ my @a :shared;
+ push @a, bless &threads::shared::share({}) for 1..1000;
+ @a = (); # this creates lots of temp SVs
+ })->join;
+ ok(1, "#131124 clear array doesnt panic");
+}
-- The optimist believes that he lives in the best of all possible worlds. |
From @khwilliamsonFixed by defb77b |
@khwilliamson - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#131124 (status was 'resolved')
Searchable as RT131124$
The text was updated successfully, but these errors were encountered: