XS modules having problems with CLONE and ithreads unless PERL_NO_GET_CONTEXT is defined
Stas Bekman
implementation of the CLONE function, to show when this function is needed,
how to write a possess method, tests, etc. Eventually it should land in the
threads.pod or around it. You can look at what I've done so far:
http://apache.org/~stas/Example-CLONE-0.01.tar.gz
Now I've discovered a problem with XS functions called from CLONE. This
problem does not appear before perl 5.8.2, but can be seen in 5.8.2, maint and
blead perls. What happens is that XSUBs called from CLONE receive the
arguments to threads->new(). So for example if you call:
threads->new(\&test);
and a CLONE function calls $bar->foo(). foo() won't receive a single argument
$bar, but two arguments: the 'threads' class and a reference to the function
test. Which seems to be a stack frames maintenance problem.
Notice however that if the XS module defines:
#define PERL_NO_GET_CONTEXT /* we want efficiency */
the problem goes away.
To demonstrate the problem, build the Example::CLONE package as you build any
other module (of course you need a threaded perl 5.8.2 or higher to see the
problem). 'make test' will pass just fine (disregard the 'Attempt to free
unreferenced scalar' warnings).
Next, comment out the first line in CLONE.xs (the define above), make and make
test. Now you will get:
t/threads....ok 5/17
Usage: Example::CLONE::_possess(ref)
If you dump the stack, you will get
CODE:
sv_dump(ST(0));
sv_dump(ST(1));
you will see the two args that were passed to threads::new(). Since
PERL_NO_GET_CONTEXT fixes the problem, it seems like the XSUB gets the wrong
aTHX? Again, the problem emerged in 5.8.2.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:st...@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Stas Bekman
> I've been working on Example::CLONE to have a fully working demo
> implementation of the CLONE function, to show when this function is
> needed, how to write a possess method, tests, etc. Eventually it should
> land in the threads.pod or around it. You can look at what I've done so
> far:
> http://apache.org/~stas/Example-CLONE-0.01.tar.gz
>
> Now I've discovered a problem with XS functions called from CLONE. This
> problem does not appear before perl 5.8.2, but can be seen in 5.8.2,
> maint and blead perls. What happens is that XSUBs called from CLONE
> receive the arguments to threads->new(). So for example if you call:
>
> threads->new(\&test);
>
> and a CLONE function calls $bar->foo(). foo() won't receive a single
> argument $bar, but two arguments: the 'threads' class and a reference to
> the function test. Which seems to be a stack frames maintenance problem.
>
> Notice however that if the XS module defines:
> #define PERL_NO_GET_CONTEXT /* we want efficiency */
> the problem goes away.
And here again my fix for -Dm segfault bites me back :( This context setting
in proto_perl affects XS modules, unless PERL_NO_GET_CONTEXT is defined.
perl_cloned perls running XSUBs from CLONE get parent perl's stack via its
context.
So I suggest to revert my change (even though 5.8.2 is unfortunately out
already), and replace it with this comment until someone will fix perlio so
that it'll be able to work without a valid perl context (which is the case in
perl_clone)
diff -u sv.c.orig sv.c
--- sv.c.orig 2003-11-13 11:27:25.000000000 -0800
+++ sv.c 2003-12-03 01:37:41.000000000 -0800
@@ -10616,9 +10616,10 @@
PL_debug = proto_perl->Idebug;
#ifdef USE_REENTRANT_API
-#ifdef DEBUGGING
- PERL_SET_CONTEXT(proto_perl);
-#endif
+ /* XXX: things like -Dm will segfault here in perlio, but doing
+ * PERL_SET_CONTEXT(proto_perl);
+ * breaks too many other things
+ */
Perl_reentrant_init(aTHX);
#endif
Rafael Garcia-Suarez
> So I suggest to revert my change (even though 5.8.2 is unfortunately out
> already), and replace it with this comment until someone will fix perlio so
> that it'll be able to work without a valid perl context (which is the case in
> perl_clone)
Thanks, applied as #21837 to bleadperl, and waiting for the next one :)
Arthur Bergman
On Wednesday, December 3, 2003, at 09:27 am, Stas Bekman wrote:
> Next, comment out the first line in CLONE.xs (the define above), make
> and make test. Now you will get:
>
> t/threads....ok 5/17
> Usage: Example::CLONE::_possess(ref)
>
> If you dump the stack, you will get
> CODE:
> sv_dump(ST(0));
> sv_dump(ST(1));
>
> you will see the two args that were passed to threads::new(). Since
> PERL_NO_GET_CONTEXT fixes the problem, it seems like the XSUB gets the
> wrong aTHX? Again, the problem emerged in 5.8.2.
>
Most likely you are correct and that the aTHX is wrong somehow.
Will investigate.
Arthur
Arthur Bergman
On Wednesday, December 3, 2003, at 09:41 am, Stas Bekman wrote:
>
> And here again my fix for -Dm segfault bites me back :( This context
> setting in proto_perl affects XS modules, unless PERL_NO_GET_CONTEXT
> is defined. perl_cloned perls running XSUBs from CLONE get parent
> perl's stack via its context.
>
> So I suggest to revert my change (even though 5.8.2 is unfortunately
> out already), and replace it with this comment until someone will fix
> perlio so that it'll be able to work without a valid perl context
> (which is the case in perl_clone)
>
>
Bah, forgot to read my emails backwards.
On another note.
WHY DOES PERLIO RELY ON HAVING A WORKING PERL? WHY CAN I NOT SIMPLE USE
PRINTF FROM THE PERL CORE ANYMORE?
WHY DOES EVERYTHING HAVE TO BE SO COMPLICATED?
Arthur
Stas Bekman
Yup, as I followed up on this, it was the proto_perl contenxt setting in
perl_clone that was causing that. I'll make sure to add a note to the CLONE
docs to advise authors using CLONE with XSUB calls to use PERL_NO_GET_CONTEXT
at least because their code won't work with 5.8.2.
Enache Adrian
Don't forget to close bug #24463 too.
Regards,
Adi
Tim Bunce
> Arthur Bergman wrote:
> >On Wednesday, December 3, 2003, at 09:27 am, Stas Bekman wrote:
> >
> >>Next, comment out the first line in CLONE.xs (the define above), make
> >>and make test. Now you will get:
> >>
> >>t/threads....ok 5/17
> >>Usage: Example::CLONE::_possess(ref)
> >>
> >>If you dump the stack, you will get
> >> CODE:
> >> sv_dump(ST(0));
> >> sv_dump(ST(1));
> >>
> >>you will see the two args that were passed to threads::new(). Since
> >>PERL_NO_GET_CONTEXT fixes the problem, it seems like the XSUB gets the
> >>wrong aTHX? Again, the problem emerged in 5.8.2.
> >
> >Most likely you are correct and that the aTHX is wrong somehow.
>
> Yup, as I followed up on this, it was the proto_perl contenxt setting in
> perl_clone that was causing that. I'll make sure to add a note to the CLONE
> docs to advise authors using CLONE with XSUB calls to use
> PERL_NO_GET_CONTEXT at least because their code won't work with 5.8.2.
That would be a big change for the DBI. Patches welcome, of course :)
Will ponie support both PERL_NO_GET_CONTEXT and non-PERL_NO_GET_CONTEXT
extensions?
Tim
Stas Bekman
[...]
>>Yup, as I followed up on this, it was the proto_perl contenxt setting in
>>perl_clone that was causing that. I'll make sure to add a note to the CLONE
>>docs to advise authors using CLONE with XSUB calls to use
>>PERL_NO_GET_CONTEXT at least because their code won't work with 5.8.2.
>
>
> That would be a big change for the DBI. Patches welcome, of course :)
Won't it also affect the DBD modules, if they ever call into DBI APIs which
may now pass pTHX_?
Luckily it's a no-brainer to do the porting. Just define PERL_NO_GET_CONTEXT
and add the pTHX_/aTHX_ args and remove dTHX if any, till it compiles.
Nick Ing-Simmons
I wanted to have fully global variables shared between threads
but Win32's per-thread-heaps killed that. So PerlIO tables etc. have
to be thread vars and cloned.
Also I am using SVs as growable buffers and not re-inventing
things. It could probably be made to work with true global tables
(Win32 does have a global heap as well) if a few things are re-implemented.
>WHY CAN I NOT SIMPLE USE
>PRINTF FROM THE PERL CORE ANYMORE?
You can, but it won't (necessarily) write to STDOUT, it will write to stdout.
i.e. it won't do utf8 or :encode translations.
>
>WHY DOES EVERYTHING HAVE TO BE SO COMPLICATED?
Mainly trying to give a consistent interfaces to wildly inconsistent
OSes (UNIX, Win32, VMS, ...)
>
>Arthur
Arthur Bergman
On Thursday, December 4, 2003, at 08:40 pm, Tim Bunce wrote:
>
> Will ponie support both PERL_NO_GET_CONTEXT and non-PERL_NO_GET_CONTEXT
> extensions?
>
>
I don't really see an option in not supporting them.
Arthur
Tim Bunce
> Tim Bunce wrote:
> [...]
> >>Yup, as I followed up on this, it was the proto_perl contenxt setting in
> >>perl_clone that was causing that. I'll make sure to add a note to the
> >>CLONE docs to advise authors using CLONE with XSUB calls to use
> >>PERL_NO_GET_CONTEXT at least because their code won't work with 5.8.2.
> >
> >
> >That would be a big change for the DBI. Patches welcome, of course :)
>
> Won't it also affect the DBD modules, if they ever call into DBI APIs which
> may now pass pTHX_?
Probably. I haven't looked. But if so that makes it a very big hurdle as it
would mean releasing a DBI that just won't work with existing compiled drivers,
even if they're recompiled.
I'd probably opt to have the DBI manually get the context in those cases.
I'll ponder this for DBI v2, which I'm pondering when time allows.
> Luckily it's a no-brainer to do the porting. Just define
> PERL_NO_GET_CONTEXT and add the pTHX_/aTHX_ args and remove dTHX if any,
> till it compiles.
Yeap. Multiplied by any #ifdef permutations lying around in the code.
But I don't have many of them.
Tim.
Arthur Bergman
This was meant as a retholical rant venting my frustration debugging
threads, not for something that needed rebuttal, sorry if you thought
so.
Arthur
Jan Dubois
>I've been working on Example::CLONE to have a fully working demo
>implementation of the CLONE function, to show when this function is needed,
>how to write a possess method, tests, etc. Eventually it should land in the
>threads.pod or around it. You can look at what I've done so far:
>http://apache.org/~stas/Example-CLONE-0.01.tar.gz
Thanks for the sample code!
You should probably also mention that every module that stores
per-interpreter data in PL_modglobal may need to "clone" it in
the CLONE method as well.
This is especially true for modules that use MY_CXT_INIT to allocate
the per-interpreter storage. It allocates the buffer as the PVX of
an untracked SV and then just stores the pointer as a UV. The SV is
freed when the interpreter is destructed.
perl_clone() will copy the UV ptr in PL_modglobal, but if the first
interpreter is destroyed before the second one, then the PL_modglobal
entry in the second interpreter points to freed memory.
I would suggest that we add this macro to perl.h:
#define MY_CXT_CLONE \
dMY_CXT_SV; \
my_cxt_t *my_cxtp = (my_cxt_t*)SvPVX(newSV(sizeof(my_cxt_t)-1));\
Copy(INT2PTR(my_cxt_t, SvUV(my_cxt_sv)), my_extp, 1, my_cxt_t); \
sv_setuv(my_cxt_sv, PTR2UV(my_cxtp))
and then recommend that each module using MY_CXT_INIT calls this macro in
an XS CLONE method such as:
void
CLONE(...)
CODE:
MY_CXT_CLONE;
I noticed that both the 're' and the 'Storable' core modules store data in
PL_modglobal. At least 're' needs to use MY_CXT_CLONE. I haven't looked
closely at 'Storable' to see if it handles interpreter cloning correctly or
not (it uses its own PL_modglobal handling macros).
I can send a patch for perl.h and re.xs (and Devel::PPPort.pm) unless someone
has a better idea how this should be handled.
Cheers,
-Jan
Stas Bekman
> On Wed, 03 Dec 2003 01:27:34 -0800, Stas Bekman <st...@stason.org> wrote:
>
>
>>I've been working on Example::CLONE to have a fully working demo
>>implementation of the CLONE function, to show when this function is needed,
>>how to write a possess method, tests, etc. Eventually it should land in the
>>threads.pod or around it. You can look at what I've done so far:
>>http://apache.org/~stas/Example-CLONE-0.01.tar.gz
>
>
> Thanks for the sample code!
;)
> You should probably also mention that every module that stores
> per-interpreter data in PL_modglobal may need to "clone" it in
> the CLONE method as well.
>
> This is especially true for modules that use MY_CXT_INIT to allocate
> the per-interpreter storage. It allocates the buffer as the PVX of
> an untracked SV and then just stores the pointer as a UV. The SV is
> freed when the interpreter is destructed.
>
> perl_clone() will copy the UV ptr in PL_modglobal, but if the first
> interpreter is destroyed before the second one, then the PL_modglobal
> entry in the second interpreter points to freed memory.
Certainly, I'll add it to the discussion.
I suppose this should be also mentioned in perlmod.pod, where the CLONE class
method is documented.
I wonder how can we integrate the documentation and sample code from the
Example-CLONE-0.01.tar.gz package into the perl core documentation where it
really belongs to. The only reason I created it as a separate package is
because it contains XS code, so Makefile.PL is needed. I suppose the build
files, the .xs and .pm files and the tests can be "flattened" into a pod.
[...]
Jan Dubois
>Jan Dubois wrote:
>> On Wed, 03 Dec 2003 01:27:34 -0800, Stas Bekman <st...@stason.org> wrote:
>>
>>
>>>I've been working on Example::CLONE to have a fully working demo
>>>implementation of the CLONE function, to show when this function is needed,
>>>how to write a possess method, tests, etc. Eventually it should land in the
>>>threads.pod or around it. You can look at what I've done so far:
>>>http://apache.org/~stas/Example-CLONE-0.01.tar.gz
>>
>>
>> Thanks for the sample code!
>
>;)
I noticed one problem with the sample code: After you insert a
reference to the updated object into %objects, shouldn't you weaken()
the new reference?
>> You should probably also mention that every module that stores
>> per-interpreter data in PL_modglobal may need to "clone" it in
>> the CLONE method as well.
[...]
>Certainly, I'll add it to the discussion.
>
>I suppose this should be also mentioned in perlmod.pod, where the CLONE class
>method is documented.
>
>I wonder how can we integrate the documentation and sample code from the
>Example-CLONE-0.01.tar.gz package into the perl core documentation where it
>really belongs to. The only reason I created it as a separate package is
>because it contains XS code, so Makefile.PL is needed. I suppose the build
>files, the .xs and .pm files and the tests can be "flattened" into a pod.
Ideally h2xs should "know" about CLONE and have an option to produce the
appropriate boilerplate. xsubpp could verify that the CLONE code has
been inserted when it detects that any of T_PTRxxx typemap entries are
being used.
Well, I don't have a good suggestion about this, but I feel the amount
of code that needs to be manually written for each module to support
proper multithreading is too big. E.g Perl could take care of the lists
of weak references etc. and then just call a CLONE_INSTANCE method for
each object automatically as part of a standard CLONE implementation.
Cheers,
-Jan
Stas Bekman
> On Tue, 30 Dec 2003 20:56:51 -0800, Stas Bekman <st...@stason.org> wrote:
>
>
>>Jan Dubois wrote:
>>
>>>On Wed, 03 Dec 2003 01:27:34 -0800, Stas Bekman <st...@stason.org> wrote:
>>>
>>>
>>>
>>>>I've been working on Example::CLONE to have a fully working demo
>>>>implementation of the CLONE function, to show when this function is needed,
>>>>how to write a possess method, tests, etc. Eventually it should land in the
>>>>threads.pod or around it. You can look at what I've done so far:
>>>>http://apache.org/~stas/Example-CLONE-0.01.tar.gz
>>>
>>>
>>>Thanks for the sample code!
>>
>>;)
>
>
> I noticed one problem with the sample code: After you insert a
> reference to the updated object into %objects, shouldn't you weaken()
> the new reference?
I think you are right. I've added that fix and your notes on PL_modglobal and
put up a new distro:
http://apache.org/~stas/Example-CLONE-0.02.tar.gz
[...]
>>Certainly, I'll add it to the discussion.
>>
>>I suppose this should be also mentioned in perlmod.pod, where the CLONE class
>>method is documented.
>>
>>I wonder how can we integrate the documentation and sample code from the
>>Example-CLONE-0.01.tar.gz package into the perl core documentation where it
>>really belongs to. The only reason I created it as a separate package is
>>because it contains XS code, so Makefile.PL is needed. I suppose the build
>>files, the .xs and .pm files and the tests can be "flattened" into a pod.
>
>
> Ideally h2xs should "know" about CLONE and have an option to produce the
> appropriate boilerplate. xsubpp could verify that the CLONE code has
> been inserted when it detects that any of T_PTRxxx typemap entries are
> being used.
I think you don't always need a typemap if you use XS() functions and do the
translation from/to SV by yourself. So h2xs may miss the need for the CLONE
code. But it's certainly better to have it detect most of cases.
> Well, I don't have a good suggestion about this, but I feel the amount
> of code that needs to be manually written for each module to support
> proper multithreading is too big. E.g Perl could take care of the lists
> of weak references etc. and then just call a CLONE_INSTANCE method for
> each object automatically as part of a standard CLONE implementation.
+1.
That's what Liz's Thread::Bless is partially trying to do. Certainly an
internal support would be the best, but it should be available for older 5.8.x
perls via CPAN module.