gisle / tcl.pm

Tcl extension module for Perl
http://search.cpan.org/dist/Tcl
9 stars 8 forks source link

Add closing tcl after/wait to clean up interpreter #4

Closed paultcochrane closed 8 years ago

paultcochrane commented 8 years ago

This change patches the effects of a bug which appears when running t/call.t, namely that at the end of the test, the process segfaults causing the entire test run to fail. It seems that by adding an extra Tcl after/wait combination allows the interpreter process to be cleaned up properly before (or maybe while) destroying the objects in Perl. The problem only manifests itself when using a Perl sub in a Tcl after call; commenting out such code removes the segfault, which itself happens in XS_Tcl_DeleteCommand, (more directly within Tcl_DeleteCommand on line 1559 of Tcl.xs).

The solution presented here was found by comparing the code in this file with that in t/disposal-subs.t which also uses an after/wait step which doesn't seem to add any value to the test (the disposal-subs test also segfaults if the extra after/wait is removed), however does stop the code from segfaulting. It is also interesting to note that when running the disposal-subs test with the after/wait commented out shows that the perl code within the ::perl:: namespace of Tcl mounts up (there are 1004 such code objects at the end of the disposal-stubs run). It could be that references aren't being cleaned up correctly, however it wasn't clear where this was happening, hence this patch over the actual issue. This change makes the tests pass on perl versions 5.8 .. 5.22.

vadrer commented 8 years ago

the segfault happening on Linux? need better way of fixing the segfault. ok, if I'll not find a better way - then I will agree with this change :)

keep in mind that the matter could be discussed on the devoted ML,

paultcochrane commented 8 years ago

Yes it's happening on Linux. I agree that a better way is needed in order to fix the segfault, however I couldn't find a solution, and the workaround I posted was based upon what already looked like a workaround in the code. I can post gdb tracebacks if that might be of help.

vadrer commented 8 years ago

thanks... if possible send the gdb traceback. are you aware of dedicated ML BTW?

paultcochrane commented 8 years ago

Here's the information from the gdb backtrace. You mentioned the mailing list in another ticket. I'm guessing you mean tcltk@perl.org?

Running t/call.t through gdb:

gdb --args perl -Iblib/lib -Iblib/arch t/call.t

gives

ok 15
[New LWP 30652]
[LWP 30652 exited]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6956ccc in XS_Tcl_DeleteCommand () from blib/arch/auto/Tcl/Tcl.so
(gdb) where
#0  0x00007ffff6956ccc in XS_Tcl_DeleteCommand () from blib/arch/auto/Tcl/Tcl.so
#1  0x00000000004a5920 in Perl_pp_entersub ()
#2  0x000000000049e753 in Perl_runops_standard ()
#3  0x0000000000435677 in Perl_call_sv ()
#4  0x00000000004ae667 in S_curse ()
#5  0x00000000004aef8d in Perl_sv_clear ()
#6  0x00000000004af31f in Perl_sv_free2 ()
#7  0x00000000004a6d52 in S_visit ()
#8  0x00000000004af72c in Perl_sv_clean_objs ()
#9  0x0000000000437848 in perl_destruct ()
#10 0x000000000041de14 in main ()
vadrer commented 8 years ago

thanks a lot. IIRC - the error could be suppressed by using more conservative way of deleting the resource, so if we'll not succeed in quickly resolving the issue - we still could release a better version which is cleaned and modernised with your help.

please do not forget to also change Changelog, and also mention yourself in authours section here there and everywhere :)

vadrer commented 8 years ago

I've found the way to fix this segfault, and then improved t/disposal-subs.t which now features similar but another segfault, will commit these changes tomorrow and will there fore discard yours :)

....and then resolved the issue, fortunately :)

vadrer commented 8 years ago

ok done :)

paultcochrane commented 8 years ago

Cool! :-)

vadrer commented 8 years ago

I''ll release a version in the middle of the next week... do you have additions to meta.yml etc so to modernize it and to avoid bogus FAIL reports in case of missing tcl and/or tk??

TIA :)