Crash when joining multiple threads
Jeff Clites
creates-then-joins a thread (looping twice), I get a crash at the end:
set I16, 0
again:
inc I16
new P5, .ParrotThread
find_global P6, "_foo"
find_method P0, P5, "thread3"
invoke
set I5, P5
getinterp P2
find_method P0, P2, "join"
invoke
lt I16, 2, again
print "done\n"
end
.pcc_sub _foo:
invoke P1
The problem is that when joining the _second_ time, in pt_thread_join()
you get a return value from joining the thread--which happens to
contain a thread-interpreter PMC:
(gdb) p (char*)((PMC*)retval)->vtable->isa_str->strstart
$2 = 0x1dc40c "ParrotThread ParrotInterpreter"
This then gets cloned, which ultimately ends up messing up the
interpreter_array[] (things end up in the wrong slot), and
pt_join_threads() ends up trying to join a bogus thread, and you get a
crash.
So, the bug seems to be that the second time through, you get a return
value from the thread. I don't know why this is happening--haven't
tried digging yet.
JEff
Leopold Toetsch
> If I run this code, which should just be a loop which
> creates-then-joins a thread (looping twice), I get a crash at the end:
> The problem is that when joining the _second_ time, in pt_thread_join()
> you get a return value from joining the thread
The return value is only returned, when I3 != 0. For your example that
shouldn't be the case (I3 is unused aka zero). So there isn't any return
value passed back.
Are you running the latest source? It doesn't crash here.
> JEff
leo
Jeff Clites
On Jan 5, 2004, at 5:32 AM, Leopold Toetsch wrote:
> Jeff Clites <jcl...@mac.com> wrote:
>> If I run this code, which should just be a loop which
>> creates-then-joins a thread (looping twice), I get a crash at the end:
>
>> The problem is that when joining the _second_ time, in
>> pt_thread_join()
>> you get a return value from joining the thread
>
> The return value is only returned, when I3 != 0. For your example that
> shouldn't be the case (I3 is unused aka zero). So there isn't any
> return
> value passed back.
Yep, the second time through I find REG_INT(3) (that is,
interpreter->int_reg.registers[3]) set to 1 in thread_func().
> Are you running the latest source? It doesn't crash here.
Yep, I just verified, using a clean checkout and build:
% gdb parrot
...
(gdb) b src/thread.c:238
Breakpoint 1 at 0xf890c: file src/thread.c, line 238.
(gdb) r thrspeed.pasm
Starting program:
/Users/jac/Projects/parrot/parrot-thread-join-crash/parrot
thrspeed.pasm
Reading symbols for shared libraries . done
Breakpoint 1, pt_thread_join (parent=0x1000200, tid=1) at
src/thread.c:238
238 if (retval) {
(gdb) p retval
$1 = (void *) 0x0
(gdb) c
Continuing.
Breakpoint 1, pt_thread_join (parent=0x1000200, tid=1) at
src/thread.c:238
238 if (retval) {
(gdb) p retval
$2 = (void *) 0x10163d0
(gdb) p (char*)((PMC*)retval)->vtable->isa_str->strstart
$3 = 0x1da410 "ParrotThread ParrotInterpreter"
(gdb) c
Continuing.
done
Program received signal EXC_BAD_ACCESS, Could not access memory.
0x90039138 in pthread_join ()
(gdb) bt
#0 0x90039138 in pthread_join ()
#1 0x000f8b78 in pt_join_threads (interpreter=0x1000200) at
src/thread.c:309
#2 0x0000aba0 in Parrot_really_destroy (exit_code=0,
vinterp=0x1000200) at src/interpreter.c:1123
#3 0x0000c120 in Parrot_exit (status=0) at src/exit.c:48
#4 0x00003544 in main (argc=1, argv=0xbffffbd4) at imcc/main.c:555
(gdb) f 1
#1 0x000f8b78 in pt_join_threads (interpreter=0x1000200) at
src/thread.c:309
309 JOIN(thread_interp->thread_data->thread, retval);
(gdb) p thread_interp->thread_data
$4 = (struct _Thread_data *) 0x903520
(gdb) p thread_interp->thread_data->thread
$5 = 0x0
And here's myconfig:
Summary of my parrot 0.0.13 configuration:
configdate='Mon Jan 5 08:33:02 2004'
Platform:
osname=darwin, archname=darwin
jitcapable=1, jitarchname=ppc-darwin,
jitosname=DARWIN, jitcpuarch=ppc
execcapable=1
perl=perl
Compiler:
cc='cc', ccflags='-g -pipe -pipe -fno-common -no-cpp-precomp
-DHAS_TELLDIR_PROTOTYPE -pipe -fno-common -Wno-long-double ',
Linker and Libraries:
ld='cc', ldflags=' -flat_namespace ',
cc_ldflags='',
libs='-lm'
Dynamic Linking:
so='.dylib', ld_shared=' -flat_namespace -bundle -undefined
suppress',
ld_shared_flags=''
Types:
iv=long, intvalsize=4, intsize=4, opcode_t=long, opcode_t_size=4,
ptrsize=4, ptr_alignment=4 byteorder=4321,
nv=double, numvalsize=8, doublesize=8
JEff
Leopold Toetsch
> On Jan 5, 2004, at 5:32 AM, Leopold Toetsch wrote:
>> The return value is only returned, when I3 != 0. For your example that
>> shouldn't be the case (I3 is unused aka zero). So there isn't any
>> return
>> value passed back.
> Yep, the second time through I find REG_INT(3) (that is,
> interpreter->int_reg.registers[3]) set to 1 in thread_func().
Wherever that comes from, just insert a line
set I3, 0
before you invoke the return continutation. That's the official way, to
inidicate a void return value.
leo
Jeff Clites
I tracked down how I3 was getting set, even though from the pasm it
looks like nothing should be changing it. It turns out to be a side
effect of the use of NCI in the threading implementation; the problem
wasn't showing up for you since you have CAN_BUILD_CALL_FRAMES defined
(but I don't). So I have a patch below which avoids setting I3 to 1 if
the return value (in P5) is null--the generated NCI stubs were setting
I3 to 1 for functions whose signatures indicate that they return PMCs,
even if there was no PMC returned. (With this patch, the script I
posted works, and the nci.t tests continue to pass.)
Also, looking at the i386 Parrot_jit_build_call_func() code I noticed
that in the case of a "P" return type, it was setting a PMC register
but incrementing the int register count. I have a patch below to fix
this also; I don't have a way to test it, though.
JEff
Index: build_tools/build_nativecall.pl
===================================================================
RCS file: /cvs/public/parrot/build_tools/build_nativecall.pl,v
retrieving revision 1.35
diff -u -r1.35 build_nativecall.pl
--- build_tools/build_nativecall.pl 3 Jan 2004 20:03:38 -0000
1.35
+++ build_tools/build_nativecall.pl 6 Jan 2004 08:35:33 -0000
@@ -314,8 +314,14 @@
sub set_return_count {
my ($stack, $int, $string, $pmc, $num) = @_;
+
+ my $pmc_string;
+
+ if( $pmc ) { $pmc_string = "return_data ? $pmc : 0" }
+ else { $pmc_string = 0 }
+
print NCI <<FOOTER;
- set_return_val(interpreter, $stack, $int, $string, $pmc, $num);
+ set_return_val(interpreter, $stack, $int, $string, $pmc_string,
$num);
return;
}
Index: jit/i386/jit_emit.h
===================================================================
RCS file: /cvs/public/parrot/jit/i386/jit_emit.h,v
retrieving revision 1.99
diff -u -r1.99 jit_emit.h
--- jit/i386/jit_emit.h 3 Jan 2004 13:22:45 -0000 1.99
+++ jit/i386/jit_emit.h 6 Jan 2004 08:37:22 -0000
@@ -3050,7 +3050,7 @@
case 'v': /* void - do nothing */
break;
case 'P':
- jit_emit_mov_mr_i(pc, &PMC_REG(next_i++), emit_EAX);
+ jit_emit_mov_mr_i(pc, &PMC_REG(next_p++), emit_EAX);
break;
case 'p': /* make a new unmanaged struct */
/* save return value on stack */
Matt Fowles
I just got a brand new checkout tonite and have the following problem:
$ perl Configure.pl
Parrot Version 0.0.13 Configure 2.0
Copyright (C) 2001-2003 The Perl Foundation. All Rights Reserved.
Hello, I'm Configure. My job is to poke and prod your system to figure out
how to build Parrot. The process is completely automated, unless you passed in
the `--ask' flag on the command line, in which case it'll prompt you for a few
pieces of info.
Since you're running this script, you obviously have Perl 5--I'll be pulling
some defaults from its configuration.
Checking MANIFEST...done.
Setting up Configure's data structures...done.
Checking for --miniparrot...done.
Loading platform and local hints files...[ config/init/hints/linux.pl ]done.
Enabling optimization...(none requested) done.
Determinig nongenerated header files...done.
Determining what C compiler and linker to use...done.
Determining what types Parrot should use...done.
Determining what opcode files should be compiled in...done.
Setting up experimental systems...done.
Determining what pmc files should be compiled in...done.
Tweaking ccflags...done.
Determining your minimum pointer alignment... not tested (4) done.
Probing for C headers...done.
Determining some sizes...Linker failed (see test.ldo) at lib/Parrot/Configure/Step.pm line 233
Parrot::Configure::Step::cc_build() called at config/auto/sizes.pl line 39
Configure::Step::runstep('undef') called at lib/Parrot/Configure/RunSteps.pm line 68
Parrot::Configure::RunSteps::runsteps('Parrot::Configure::RunSteps','debugging',1) called at Configure.pl line 94
$ cat test.ldo
/usr/bin/ld: cannot find -lgdbm_compat
collect2: ld returned 1 exit status
Hope someone know more about this gdbm_compat thing then I do...
I am running a relatively stock Debian testing box.
Hope this helps,
Matt
Lars Balker Rasmussen
> $ cat test.ldo
> /usr/bin/ld: cannot find -lgdbm_compat
> collect2: ld returned 1 exit status
>
>
> Hope someone know more about this gdbm_compat thing then I do...
>
> I am running a relatively stock Debian testing box.
Changing line 27 of config/inter/progs.pl to
grep { $^O=~/VMS|MSWin/ || !/^-l(c|gdbm(_compat)?|dbm|ndbm|db)$/ }
should help you along.
I don't have a Debian box, but a friends "revision 5.0 version 8
subversion 2 (Debian unstable)" reveals:
% perl -le 'use Config; print $Config{libs}'
-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
I have no idea why Debians perl is linked with gdbm_compat as well as
gdbm.
I suppose this is a patch :-)
--
Lars Balker Rasmussen Consult::Perl
Leopold Toetsch
> I tracked down how I3 was getting set, even though from the pasm it
> looks like nothing should be changing it. It turns out to be a side
> effect of the use of NCI in the threading implementation; the problem
> wasn't showing up for you since you have CAN_BUILD_CALL_FRAMES defined
> (but I don't). So I have a patch below which avoids setting I3 to 1 if
> the return value (in P5) is null--the generated NCI stubs were setting
> I3 to 1 for functions whose signatures indicate that they return PMCs,
> even if there was no PMC returned. (With this patch, the script I
> posted works, and the nci.t tests continue to pass.)
I'm not sure, if your patch is totally correct, because the P5 register
is still changed. Its the problem if we should trust the signature
(which has a P-return value) or the value of I3 from the return
signature.
Anyway, applied
Thanks,
leo