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

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

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@paultcochrane
Contributor

paultcochrane commented Jan 2, 2016

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.

Add closing tcl after/wait to clean up interpreter
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

This comment has been minimized.

Show comment
Hide comment
@vadrer

vadrer Jan 8, 2016

Collaborator

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,

Collaborator

vadrer commented Jan 8, 2016

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

This comment has been minimized.

Show comment
Hide comment
@paultcochrane

paultcochrane Jan 8, 2016

Contributor

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.

Contributor

paultcochrane commented Jan 8, 2016

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

This comment has been minimized.

Show comment
Hide comment
@vadrer

vadrer Jan 9, 2016

Collaborator

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

Collaborator

vadrer commented Jan 9, 2016

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

@paultcochrane

This comment has been minimized.

Show comment
Hide comment
@paultcochrane

paultcochrane Jan 11, 2016

Contributor

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 ()
Contributor

paultcochrane commented Jan 11, 2016

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

This comment has been minimized.

Show comment
Hide comment
@vadrer

vadrer Jan 12, 2016

Collaborator

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 :)

Collaborator

vadrer commented Jan 12, 2016

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 vadrer self-assigned this Jan 12, 2016

@paultcochrane paultcochrane referenced this pull request Jan 13, 2016

Merged

Update changelog #9

@vadrer

This comment has been minimized.

Show comment
Hide comment
@vadrer

vadrer Jan 13, 2016

Collaborator

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 :)

Collaborator

vadrer commented Jan 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@vadrer

vadrer Jan 13, 2016

Collaborator

ok done :)

Collaborator

vadrer commented Jan 13, 2016

ok done :)

@vadrer vadrer closed this Jan 13, 2016

@paultcochrane

This comment has been minimized.

Show comment
Hide comment
@paultcochrane

paultcochrane Jan 24, 2016

Contributor

Cool! :-)

Contributor

paultcochrane commented Jan 24, 2016

Cool! :-)

@paultcochrane paultcochrane deleted the paultcochrane:pr/fix-call-test-segfault branch Jan 24, 2016

@vadrer

This comment has been minimized.

Show comment
Hide comment
@vadrer

vadrer Feb 6, 2016

Collaborator

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 :)

Collaborator

vadrer commented Feb 6, 2016

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment