Skip Menu |
 

This queue is for tickets about the Tcl CPAN distribution.

Report information
The Basics
Id: 125472
Status: open
Priority: 0/
Queue: Tcl

People
Owner: Nobody in particular
Requestors: huck [...] finn.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: error with coderefs passes as scalar args
Date: Mon, 04 Jun 2018 01:47:34 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Distribution name and version Tcl-1.05 Perl version/Os perl 5, version 18, subversion 2 (v5.18.2) built for i686-linux-gnu-thread-multi-64int Linux O170Ltemp 3.13.0-142-generic #191-Ubuntu SMP Fri Feb 2 12:14:37 UTC 2018 i686 i686 i686 GNU/Linux ubuntu 14.04 perl 5, version 26, subversion 1 (v5.26.1) built for i686-linux-gnu-thread-multi-64int Linux gx620-gw 4.15.0-20-generic #21-Ubuntu SMP Tue Apr 24 06:15:38 UTC 2018 i686 i686 i686 GNU/Linux ubuntu 18.04 perl 5, version 22, subversion 1 (v5.22.1) built for i686-linux-gnu-thread-multi-64int Linux ube520 4.4.0-124-generic #148-Ubuntu SMP Wed May 2 13:01:51 UTC 2018 i686 i686 i686 GNU/Linux ubuntu 16.04 More history at http://www.perlmonks.org/?node_id=1215721 I had a problem where i got an error box in my Tkx program that said invalid command name "::perl::CODE(0x96f045c)" and it didnt execute my Tkx::after command As mentioned in the perlmonks post i tracked it down to how Tcl.pm deals with CODE refs via $anon_refs and CreateCommand/DeleteCommand in create_tcl_sub. and Tcl::Code::DESTROY I think i fixed it. Follows is the diff at version 1.05 384a385 Show quoted text
> use Scalar::Util qw(weaken);
560a562,567 Show quoted text
> # this kills the weakened tclname ref leaveng the strong current_r
ref (like before) Show quoted text
> > if (exists($anon_refs{$current_r})){ > anon_kill($anon_refs{$current_r}[2]); > } >
594a602,607 Show quoted text
> # kill an anon_refs name > sub anon_kill { > my $anonname=shift; > delete $anon_refs{$anonname}; > } >
618c631,639 < $anon_refs{$rname} = bless [\$sub, $interp], 'Tcl::Code'; --- Show quoted text
> # $anon_refs{$rname} = bless [\$sub, $interp], 'Tcl::Code'; > my $isnew=0; > unless (exists $anon_refs{$tclname}) { > $anon_refs{$tclname} = bless [\$sub, $interp, $tclname], 'Tcl::Code'; > $isnew=1; > } > $anon_refs{$rname} = $anon_refs{$tclname}; > weaken($anon_refs{$tclname}) if ($isnew); >
652a674 Show quoted text
> my $anonname = $_[0]->[2];
655a678 Show quoted text
> Tcl::anon_kill($anonname);
at the perlmonks link there are two failing Tkx sample programs one at the top, and another near the bottom. Enclosed below is a more simple failure Thank you for all you do for us! #### Example Tcl failure program : #### cat tcl-fail.pl use strict; use warnings; use Tcl; my $inter=Tcl->new(); my @queue; for my $ii (1..10) {push @queue,$ii.' '.'-'x10; } my $sub; $sub=sub{ return unless (scalar(@queue)); my $line =shift @queue; print $line."\n"; return unless (scalar(@queue)); $inter->call('after',200,$sub); }; $inter->call('after',200,$sub); $inter->icall('after', 3000, 'set var fafafa'); $inter->icall('vwait', 'var'); # will wait for 3 seconds exit; ##### example failing: ##### perl tcl-fail.pl 1 ---------- 2 ---------- 3 ---------- 4 ---------- 5 ---------- invalid command name "::perl::CODE(0x12a7c58)" while executing "::perl::CODE(0x12a7c58)" ("after" script) #### example after patch above ##### perl tcl-fail.pl 1 ---------- 2 ---------- 3 ---------- 4 ---------- 5 ---------- 6 ---------- 7 ---------- 8 ---------- 9 ---------- 10 ----------
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Mon, 04 Jun 2018 02:37:27 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 1.4k
heh , "Show quoted text ", hope this is better diff -u1 /usr/local/lib/perl/5.18.2/Tcl.pm.bak /usr/local/lib/perl/5.18.2/Tcl.pm --- /usr/local/lib/perl/5.18.2/Tcl.pm.bak 2016-06-28 12:02:41.000000000 -0500 +++ /usr/local/lib/perl/5.18.2/Tcl.pm 2018-06-04 01:00:28.974837103 -0500 @@ -384,2 +384,3 @@ use strict; +use Scalar::Util qw(weaken); @@ -560,2 +561,8 @@ } +# this kills the weakened tclname ref leaveng the strong current_r ref (like before) + + if (exists($anon_refs{$current_r})){ + anon_kill($anon_refs{$current_r}[2]); + } + @@ -594,2 +601,8 @@ +# kill an anon_refs name +sub anon_kill { + my $anonname=shift; + delete $anon_refs{$anonname}; + } + # create_tcl_sub will create TCL sub that will invoke perl CODE ref @@ -617,3 +630,11 @@ # Tcl::Code::DESTROY - $anon_refs{$rname} = bless [\$sub, $interp], 'Tcl::Code'; +# $anon_refs{$rname} = bless [\$sub, $interp], 'Tcl::Code'; + my $isnew=0; + unless (exists $anon_refs{$tclname}) { + $anon_refs{$tclname} = bless [\$sub, $interp, $tclname], 'Tcl::Code'; + $isnew=1; + } + $anon_refs{$rname} = $anon_refs{$tclname}; + weaken($anon_refs{$tclname}) if ($isnew); + @@ -652,2 +673,3 @@ my $interp = $_[0]->[1]; + my $anonname = $_[0]->[2]; my $tclname = "::perl::$$rsub"; @@ -655,2 +677,3 @@ $interp->DeleteCommand($tclname) if defined $interp; + Tcl::anon_kill($anonname); }
Subject: [rt.cpan.org #125472] error with coderefs passes as scalar args (patch explanation)
Date: Mon, 04 Jun 2018 11:42:05 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 2.1k
Since for some reason i felt a ned to describe the mechanisms behind my patch on perlmonks http://www.perlmonks.org/?node_id=1215859 i thought i would include it here too This issue is only related to after, and involves code modified in https://github.com/gisle/tcl.pm/commit/7cb5f5d40ca335a046394de7ef01c528356dc624#diff-ee07373df0b8077358fdb27ea461c8f9 (code disposal works, only 1 question remains; perl::Eval now in Tcl__new ) and added in the version 1.03 release as of 2016-02-20 https://metacpan.org/changes/release/VKON/Tcl-1.05 . I suspect someone (vadrer?) saw that the commands-table/$anon_refs were filling up with address to single use anon-subrefs from the "after command" and wanted to clean them up. His approach while admirable hosed calls where the subref passed into after was not a single use anon-subref (or of the [$coderef] variety, since those generated a single use anon coderef too) because it prematurely discarded them from the Commands-table/$anon_refs. While i have faith in my patch, and think it will solve the introduced problem while retaining the needed "code disposal", patching code is never as simple as it looks. What it does is introduce a "weakened" version of the coderef in the anon_refs table under the $tclname key it used to have pre v1.03 and copies that entry into a "strong" newer $current_r style name when needed, so it can be used in the "consolidated" $anon_refs that get scheduled for deletion. Now further "after" calls will be able to create their own consolidated entries for deletion but since the refcount will be greater than one for a "static" coderef since there will be multiple copies in multiple "consolidated" $anon_refs, when _code_disposal is called the Tcl::Code::DESTROY that purges that entry from the command table wont be invoked until the last consolidated $anon_refs entry has been destroyed. Since code disposal is only invoked on "after" calls, for non-after calls i dispose of the weakened $tclname key right away, and those coderefs continue to live in the commands-table/$anon_refs "forever", just like they used to pre v1.03 and still currently do. Again thanks for all you do for us!!
Download (untitled) / with headers
text/plain 321b
TYVM! will apply it today, together with other your addition; please be aware of 1, tcltk at perl.org is mailing list for the module, where you can discuss all such findings; and 2, https://github.com/gisle/tcl.pm is a repository, where you preferably send your commits; (not necessarily, I'll accept your change anyway)
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Fri, 08 Jun 2018 11:48:53 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 696b
If it is not too late you may want to hold off for a touch. I have discovered two more issues that i am currently dealing with as well and will get back to the list asap with a resolution. both involve garbage collection of the coderef and increasing PVSV refcounts. At 10:16 AM 6/8/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >TYVM! >will apply it today, together with other your addition; >please be aware of 1, tcltk at perl.org is mailing list for the >module, where you can discuss all such findings; and 2, >https://github.com/gisle/tcl.pm is a repository, where you >preferably send your commits; (not necessarily, I'll accept your change anyway)
Subject: Re: [rt.cpan.org #125472] error with coderefs passed as scalar args
Date: Thu, 14 Jun 2018 23:14:34 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 4.1k
Ok I think im ready to have a go at this. enclosed is e patch files, code-disposal-fix-v1.05.patch fixes this issue it was generated by git format-patch --stdout -1 > ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch and i am able to apply it by git am ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch Like i said im new at git, so the second file memloss1.02.patch (which is for https://rt.cpan.org/Public/Bug/Display.html?id=125577 ) is enclosed so you can see the way i did it, and the pretty lines i added lets assume those files are in the dir ~/cvs/pmlib-linkpatch and there is a dir at ~/cvs/gits/github so you run cd ~/cvs/gits/github rm -rf ~/cvs/gits/github/test-tcl git clone https://github.com/gisle/tcl.pm.git test-tcl -v cd ~/cvs/gits/github/test-tcl git checkout master git tag -a 1.02 3f5eb527e1ba3b883c2ba197e4bdaf28d940f389 -m "Release 1.02" git tag -a 1.05 333cc79bb194c7007f08060a2e872789d4604217 -m "Release 1.05" git tag -a last 94ead33b9a30f18954bbb8168343554c1ea3c20c -m 'last i know' git checkout -b memloss1.02 1.02 git am ~/cvs/pmlib-linkpatch/memloss1.02.patch now if you use gitk you see your at v1.02 with the patch applied and you can see that it works at v1.02 perl Makefile.PL make make test notice it didnt run t/disposal-subs.t, it didnt exist then, so you go on git checkout master git merge -m 'RT 125577 Tcl.xs memory leak fix' memloss1.02 perl Makefile.PL make make test and you see it works at the current master , and runs t/disposal-subs.t, cuz its current NOW , so you go on git checkout -b code-disposal-fix-v1.05 last git am ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch perl Makefile.PL make make test and see it works patched to my base , notice it doesnt run t/memleak-a.t because it didnt exist on this branch yet, but it ran all the disposal* tests, . so you go on git checkout master git merge -m 'RT 125472 Tcl.pm code disposal updates' code-disposal-fix-v1.05 perl Makefile.PL make make test Notice it ran all the disposal* and memlean* tests, its up to date!! Now run gitk to see the funny squigly lines in many colors, that shows how this all went down now what surprised me was that if you run cd ~/cvs/gits/github rm -rf ~/cvs/gits/github/strp-tcl git clone https://github.com/gisle/tcl.pm.git strp-tcl cd ~/cvs/gits/github/strp-tcl git tag -a 1.02 3f5eb527e1ba3b883c2ba197e4bdaf28d940f389 -m "Release 1.02" git tag -a 1.05 333cc79bb194c7007f08060a2e872789d4604217 -m "Release 1.05" git checkout master git am ~/cvs/pmlib-linkpatch/memloss1.02.patch git am ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch diff Tcl.pm ~/cvs/gits/github/test-tcl/Tcl.pm diff Tcl.xs ~/cvs/gits/github/test-tcl/Tcl.xs notice that the diffs are empty, yet in gitk the branching is much cleaner. so probably just running the 2 git am commands is fine, why, i-dunno, git has some deep magic to it in those patch files i dont grok yet. i went a while getting the git-am calls to make the pretty lines i wanted to be sure it was doing what i wanted. I got scared when i saw it all in one line like that. but i am happy now that both methods produce the same result, just less pretty lines. I also spent too much time chasing whitespace errors, git-am didnt like them. That changed some lines that i didnt even put in, not a lot. Probably spent too much time on the pod and making things pretty too, eh. It works how i think it should work, The code disposal routines seem to work ok, see the t/disposal-* tests for some comments about the problems i faced. If you can still bust it id like to know. A lot of the fix is additions to the pod, i hope that was ok too. This started when a Tkx program i had didnt work under 1.05, i made a stripped down version of it to get at the issue. Both used a lot of after commands for timeouts, keepalives, and sending data with a delay. They didnt like 1.05. now with these patches they are working again, and even better than before! I hope this didnt cause new problems, but if it has, let me know and ill look into it more. Again thanks for what you do and for putting up with me. **Probably spent too much time getting to know git, but now that i know more i think its worth it, its kinda slick.

Message body is not shown because sender requested not to inline it.

Download memloss1.02.patch
text/x-diff 3.5k

Message body is not shown because sender requested not to inline it.

CC: "tcltk [...] perl.org" <tcltk [...] perl.org>
Subject: RE: [rt.cpan.org #125472] error with coderefs passed as scalar args
Date: Fri, 15 Jun 2018 20:52:04 +0000
To: "bug-Tcl [...] rt.cpan.org" <bug-Tcl [...] rt.cpan.org>, "huck [...] finn.com" <huck [...] finn.com>
From: "Konovalov, Vadim" <Vadim.Konovalov [...] dell.com>
Download (untitled) / with headers
text/plain 5.1k
Hi Huck, Thank you very much for the patch, I will apply it tomorrow; Right now I have this error: konovv@RUENKONOVVL2C /cygdrive/c/vad/perl-dev/tcl.pm $ git am ../tcl.pm-patches/code-disposal-fix-v1.05.patch Applying: RT 125472 Tcl.pm code disposal updates error: Tcl.pm: does not match index Patch failed at 0001 RT 125472 Tcl.pm code disposal updates When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". But I will apply manually and soon after that release Regards, Vadim Show quoted text
-----Original Message----- From: huck via RT [mailto:bug-Tcl@rt.cpan.org] Sent: Friday, June 15, 2018 7:15 AM Subject: Re: [rt.cpan.org #125472] error with coderefs passed as scalar args Queue: Tcl Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > Ok I think im ready to have a go at this. enclosed is e patch files, code-disposal-fix-v1.05.patch fixes this issue it was generated by git format-patch --stdout -1 > ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch and i am able to apply it by git am ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch Like i said im new at git, so the second file memloss1.02.patch (which is for https://rt.cpan.org/Public/Bug/Display.html?id=125577 ) is enclosed so you can see the way i did it, and the pretty lines i added lets assume those files are in the dir ~/cvs/pmlib-linkpatch and there is a dir at ~/cvs/gits/github so you run cd ~/cvs/gits/github rm -rf ~/cvs/gits/github/test-tcl git clone https://github.com/gisle/tcl.pm.git test-tcl -v cd ~/cvs/gits/github/test-tcl git checkout master git tag -a 1.02 3f5eb527e1ba3b883c2ba197e4bdaf28d940f389 -m "Release 1.02" git tag -a 1.05 333cc79bb194c7007f08060a2e872789d4604217 -m "Release 1.05" git tag -a last 94ead33b9a30f18954bbb8168343554c1ea3c20c -m 'last i know' git checkout -b memloss1.02 1.02 git am ~/cvs/pmlib-linkpatch/memloss1.02.patch now if you use gitk you see your at v1.02 with the patch applied and you can see that it works at v1.02 perl Makefile.PL make make test notice it didnt run t/disposal-subs.t, it didnt exist then, so you go on git checkout master git merge -m 'RT 125577 Tcl.xs memory leak fix' memloss1.02 perl Makefile.PL make make test and you see it works at the current master , and runs t/disposal-subs.t, cuz its current NOW , so you go on git checkout -b code-disposal-fix-v1.05 last git am ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch perl Makefile.PL make make test and see it works patched to my base , notice it doesnt run t/memleak-a.t because it didnt exist on this branch yet, but it ran all the disposal* tests, . so you go on git checkout master git merge -m 'RT 125472 Tcl.pm code disposal updates' code-disposal-fix-v1.05 perl Makefile.PL make make test Notice it ran all the disposal* and memlean* tests, its up to date!! Now run gitk to see the funny squigly lines in many colors, that shows how this all went down now what surprised me was that if you run cd ~/cvs/gits/github rm -rf ~/cvs/gits/github/strp-tcl git clone https://github.com/gisle/tcl.pm.git strp-tcl cd ~/cvs/gits/github/strp-tcl git tag -a 1.02 3f5eb527e1ba3b883c2ba197e4bdaf28d940f389 -m "Release 1.02" git tag -a 1.05 333cc79bb194c7007f08060a2e872789d4604217 -m "Release 1.05" git checkout master git am ~/cvs/pmlib-linkpatch/memloss1.02.patch git am ~/cvs/pmlib-linkpatch/code-disposal-fix-v1.05.patch diff Tcl.pm ~/cvs/gits/github/test-tcl/Tcl.pm diff Tcl.xs ~/cvs/gits/github/test-tcl/Tcl.xs notice that the diffs are empty, yet in gitk the branching is much cleaner. so probably just running the 2 git am commands is fine, why, i-dunno, git has some deep magic to it in those patch files i dont grok yet. i went a while getting the git-am calls to make the pretty lines i wanted to be sure it was doing what i wanted. I got scared when i saw it all in one line like that. but i am happy now that both methods produce the same result, just less pretty lines. I also spent too much time chasing whitespace errors, git-am didnt like them. That changed some lines that i didnt even put in, not a lot. Probably spent too much time on the pod and making things pretty too, eh. It works how i think it should work, The code disposal routines seem to work ok, see the t/disposal-* tests for some comments about the problems i faced. If you can still bust it id like to know. A lot of the fix is additions to the pod, i hope that was ok too. This started when a Tkx program i had didnt work under 1.05, i made a stripped down version of it to get at the issue. Both used a lot of after commands for timeouts, keepalives, and sending data with a delay. They didnt like 1.05. now with these patches they are working again, and even better than before! I hope this didnt cause new problems, but if it has, let me know and ill look into it more. Again thanks for what you do and for putting up with me. **Probably spent too much time getting to know git, but now that i know more i think its worth it, its kinda slick.
Subject: RE: [rt.cpan.org #125472] error with coderefs passed as scalar args
Date: Fri, 15 Jun 2018 16:53:30 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 1.1k
At 03:53 PM 6/15/2018, you wrote: Show quoted text
>Thank you very much for the patch, I will apply it tomorrow;
Thank you!! Show quoted text
>Right now I have this error:
these two pages seem to discuss this problem https://stackoverflow.com/questions/3575712/what-to-do-if-git-am-fails-with-does-not-match-index http://git.661346.n2.nabble.com/quot-git-apply-check-quot-successes-but-git-am-says-quot-does-not-match-index-quot-td6684646.html It worked for me because i did it on clean clones, as they discuss you must have a "dirty index". One solution discussed would be git update-index --refresh git am ../tcl.pm-patches/code-disposal-fix-v1.05.patch or if you have push rights to github you can patch to a "clean copy" like i did cd /cygdrive/c/vad/perl-dev rm -rf /cygdrive/c/vad/perl-dev/cleanam-tcl.pm git clone https://github.com/gisle/tcl.pm.git cleanam-tcl.pm cd /cygdrive/c/vad/perl-dev/cleanam-tcl.pm git checkout master # to run both at once # and also fix https://rt.cpan.org/Public/Bug/Display.html?id=125577 #uncomment the below line #git am ../tcl.pm-patches/memloss1.02.patch git am ../tcl.pm-patches/code-disposal-fix-v1.05.patch git push Show quoted text
>But I will apply manually and soon after that release
Thanks again!
Subject: RE: [rt.cpan.org #125472] error with coderefs passed as scalar args
Date: Sat, 16 Jun 2018 13:47:17 +0000
To: "bug-Tcl [...] rt.cpan.org" <bug-Tcl [...] rt.cpan.org>
From: "Konovalov, Vadim" <Vadim.Konovalov [...] dell.com>
Download (untitled) / with headers
text/plain 1.5k
It was my stupid error; TYVM! Now applied; anything else? :) Show quoted text
-----Original Message----- From: huck via RT [mailto:bug-Tcl@rt.cpan.org] Sent: Saturday, June 16, 2018 12:56 AM Subject: RE: [rt.cpan.org #125472] error with coderefs passed as scalar args Queue: Tcl Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > At 03:53 PM 6/15/2018, you wrote:
>Thank you very much for the patch, I will apply it tomorrow;
Thank you!!
>Right now I have this error:
these two pages seem to discuss this problem https://stackoverflow.com/questions/3575712/what-to-do-if-git-am-fails-with-does-not-match-index http://git.661346.n2.nabble.com/quot-git-apply-check-quot-successes-but-git-am-says-quot-does-not-match-index-quot-td6684646.html It worked for me because i did it on clean clones, as they discuss you must have a "dirty index". One solution discussed would be git update-index --refresh git am ../tcl.pm-patches/code-disposal-fix-v1.05.patch or if you have push rights to github you can patch to a "clean copy" like i did cd /cygdrive/c/vad/perl-dev rm -rf /cygdrive/c/vad/perl-dev/cleanam-tcl.pm git clone https://github.com/gisle/tcl.pm.git cleanam-tcl.pm cd /cygdrive/c/vad/perl-dev/cleanam-tcl.pm git checkout master # to run both at once # and also fix https://rt.cpan.org/Public/Bug/Display.html?id=125577 #uncomment the below line #git am ../tcl.pm-patches/memloss1.02.patch git am ../tcl.pm-patches/code-disposal-fix-v1.05.patch git push
>But I will apply manually and soon after that release
Thanks again!
Subject: RE: [rt.cpan.org #125472] error with coderefs passed as scalar args
Date: Sun, 17 Jun 2018 17:02:58 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 490b
At 08:48 AM 6/16/2018, you wrote: Show quoted text
>Now applied; anything else? :)
Since are offering :) as i searched thru gitk i found those yellow cpan release tags very helpful git tag -a 1.02 3f5eb527e1ba3b883c2ba197e4bdaf28d940f389 -m "Release 1.02" git tag -a 1.05 333cc79bb194c7007f08060a2e872789d4604217 -m "Release 1.05" git push origin 1.02 git push origin 1.05 would sortof bring them more up to date. i started referring to them so much i added them to my local repository thanks again
Subject: RE: [rt.cpan.org #125472] error with coderefs passed as scalar args
Date: Tue, 19 Jun 2018 08:27:32 +0000
To: "bug-Tcl [...] rt.cpan.org" <bug-Tcl [...] rt.cpan.org>
From: "Konovalov, Vadim" <Vadim.Konovalov [...] dell.com>
Download (untitled) / with headers
text/plain 813b
Ok, done that, Thanks! Show quoted text
-----Original Message----- From: huck via RT [mailto:bug-Tcl@rt.cpan.org] Sent: Monday, June 18, 2018 1:03 AM Subject: RE: [rt.cpan.org #125472] error with coderefs passed as scalar args Queue: Tcl Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > At 08:48 AM 6/16/2018, you wrote:
>Now applied; anything else? :)
Since are offering :) as i searched thru gitk i found those yellow cpan release tags very helpful git tag -a 1.02 3f5eb527e1ba3b883c2ba197e4bdaf28d940f389 -m "Release 1.02" git tag -a 1.05 333cc79bb194c7007f08060a2e872789d4604217 -m "Release 1.05" git push origin 1.02 git push origin 1.05 would sortof bring them more up to date. i started referring to them so much i added them to my local repository thanks again
Download (untitled) / with headers
text/plain 278b
Hi, I have tried and installed the newly released Tcl-1.06, I found there causes an extra 'make test' error on Tkx module. Please check attached report. There are two errors. The first error (from t/tcl-callback.t) might be related to the patches from this ticket. FYI. SJ
Subject: Tkx_error_report.txt
PERL_DL_NONLAZY=1 "/usr/bin/perl.exe" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/LabEntry.t ...... ok t/mega-config.t ... ok t/mega.t .......... ok t/nul-char.t ...... ok Tcl error 'invalid command name "::perl::CODE(0x600079240)" at /usr/local/lib/perl5/site_perl/5.26/x86_64-cygwin-threads/Tcl.pm line 765. ' while invoking scalar result call: "eval [set foo] a {b c}" at t/tcl-callback.t line 15. t/tcl-callback.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 6/7 subtests # Test 18 got: "Tcl error 'Foo at /usr/local/lib/perl5/site_perl/5.26/x86_64-cygwin-threads/Tcl.pm line 765.\n' while invoking scalar result call:\n\t\"error Foo\" at t/tcl.t line 38.\n" (t/tcl.t at line 39) # Expected: "Foo at t/tcl.t line 38.\n" # t/tcl.t line 39 is: ok($@, "Foo at @{[__FILE__]} line @{[__LINE__ - 1]}.\n"); t/tcl.t ........... All 17 subtests passed t/tk.t ............ ok t/utf8.t .......... ok Test Summary Report ------------------- t/tcl-callback.t (Wstat: 512 Tests: 1 Failed: 0) Non-zero exit status: 2 Parse errors: Bad plan. You planned 7 tests but ran 1. t/tcl.t (Wstat: 0 Tests: 18 Failed: 1) Failed test: 18 Parse errors: Bad plan. You planned 17 tests but ran 18. Files=8, Tests=48, 19 wallclock secs ( 0.14 usr 0.06 sys + 1.81 cusr 2.85 csys = 4.86 CPU) Result: FAIL Failed 2/8 test programs. 1/48 subtests failed. make: *** [Makefile:892: test_dynamic] Error 255
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Sun, 24 Jun 2018 00:25:37 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 2.2k
Yes it is related. I have a fix for it already but i want to research farther to make sure it is sufficient. Please be patient! At 09:20 PM 6/23/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >Hi, > > I have tried and installed the newly released Tcl-1.06, I found > there causes an extra 'make test' error on Tkx module. Please check > attached report. There are two errors. The first error (from > t/tcl-callback.t) might be related to the patches from this ticket. FYI. > >SJ > > >Content-Type: text/plain; charset="utf-8"; name="Tkx_error_report.txt" >Content-Disposition: inline; filename="Tkx_error_report.txt" >Content-Transfer-Encoding: 7bit >RT-Attachment: 125472/1792538/964058 > >PERL_DL_NONLAZY=1 "/usr/bin/perl.exe" "-MExtUtils::Command::MM" >"-MTest::Harness" "-e" "undef *Test::Harness::Switches; >test_harness(0, 'blib/lib', 'blib/arch')" t/*.t >t/LabEntry.t ...... ok >t/mega-config.t ... ok >t/mega.t .......... ok >t/nul-char.t ...... ok >Tcl error 'invalid command name "::perl::CODE(0x600079240)" at >/usr/local/lib/perl5/site_perl/5.26/x86_64-cygwin-threads/Tcl.pm line 765. >' while invoking scalar result call: > "eval [set foo] a {b c}" at t/tcl-callback.t line 15. >t/tcl-callback.t .. >Dubious, test returned 2 (wstat 512, 0x200) >Failed 6/7 subtests ># Test 18 got: "Tcl error 'Foo at >/usr/local/lib/perl5/site_perl/5.26/x86_64-cygwin-threads/Tcl.pm >line 765.\n' while invoking scalar result call:\n\t\"error Foo\" at >t/tcl.t line 38.\n" (t/tcl.t at line 39) ># Expected: "Foo at t/tcl.t line 38.\n" ># t/tcl.t line 39 is: ok($@, "Foo at @{[__FILE__]} line @{[__LINE__ >- 1]}.\n"); >t/tcl.t ........... >All 17 subtests passed >t/tk.t ............ ok >t/utf8.t .......... ok > >Test Summary Report >------------------- >t/tcl-callback.t (Wstat: 512 Tests: 1 Failed: 0) > Non-zero exit status: 2 > Parse errors: Bad plan. You planned 7 tests but ran 1. >t/tcl.t (Wstat: 0 Tests: 18 Failed: 1) > Failed test: 18 > Parse errors: Bad plan. You planned 17 tests but ran 18. >Files=8, Tests=48, 19 wallclock secs ( 0.14 usr 0.06 sys + 1.81 >cusr 2.85 csys = 4.86 CPU) >Result: FAIL >Failed 2/8 test programs. 1/48 subtests failed. >make: *** [Makefile:892: test_dynamic] Error 255
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Thu, 28 Jun 2018 17:45:11 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 2.6k
ok i have the fix ready. I backed out a portion of what i had added to try to keep the $anon_refs hash clean. I decided i did not know enuf about Tcl to insure the patch would only be needed for the set verb. I also fixed some stuff about "user tracking", (which calls are using a tclname) and updated the pod a little patch is enclosed to allow git checkout master git am ../tcl.pm-patches/c-set1.06.patch i tested it against the most up-to-date today masters from github at https://github.com/gisle/tcl.pm.git https://github.com/gisle/tkx.git https://github.com/eserte/perl-tk.git At 09:20 PM 6/23/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >Hi, > > I have tried and installed the newly released Tcl-1.06, I found > there causes an extra 'make test' error on Tkx module. Please check > attached report. There are two errors. The first error (from > t/tcl-callback.t) might be related to the patches from this ticket. FYI. > >SJ > > >Content-Type: text/plain; charset="utf-8"; name="Tkx_error_report.txt" >Content-Disposition: inline; filename="Tkx_error_report.txt" >Content-Transfer-Encoding: 7bit >RT-Attachment: 125472/1792538/964058 > >PERL_DL_NONLAZY=1 "/usr/bin/perl.exe" "-MExtUtils::Command::MM" >"-MTest::Harness" "-e" "undef *Test::Harness::Switches; >test_harness(0, 'blib/lib', 'blib/arch')" t/*.t >t/LabEntry.t ...... ok >t/mega-config.t ... ok >t/mega.t .......... ok >t/nul-char.t ...... ok >Tcl error 'invalid command name "::perl::CODE(0x600079240)" at >/usr/local/lib/perl5/site_perl/5.26/x86_64-cygwin-threads/Tcl.pm line 765. >' while invoking scalar result call: > "eval [set foo] a {b c}" at t/tcl-callback.t line 15. >t/tcl-callback.t .. >Dubious, test returned 2 (wstat 512, 0x200) >Failed 6/7 subtests ># Test 18 got: "Tcl error 'Foo at >/usr/local/lib/perl5/site_perl/5.26/x86_64-cygwin-threads/Tcl.pm >line 765.\n' while invoking scalar result call:\n\t\"error Foo\" at >t/tcl.t line 38.\n" (t/tcl.t at line 39) ># Expected: "Foo at t/tcl.t line 38.\n" ># t/tcl.t line 39 is: ok($@, "Foo at @{[__FILE__]} line @{[__LINE__ >- 1]}.\n"); >t/tcl.t ........... >All 17 subtests passed >t/tk.t ............ ok >t/utf8.t .......... ok > >Test Summary Report >------------------- >t/tcl-callback.t (Wstat: 512 Tests: 1 Failed: 0) > Non-zero exit status: 2 > Parse errors: Bad plan. You planned 7 tests but ran 1. >t/tcl.t (Wstat: 0 Tests: 18 Failed: 1) > Failed test: 18 > Parse errors: Bad plan. You planned 17 tests but ran 18. >Files=8, Tests=48, 19 wallclock secs ( 0.14 usr 0.06 sys + 1.81 >cusr 2.85 csys = 4.86 CPU) >Result: FAIL >Failed 2/8 test programs. 1/48 subtests failed. >make: *** [Makefile:892: test_dynamic] Error 255
Download c-set1.06.patch
text/x-diff 9.8k

Message body is not shown because sender requested not to inline it.

thanks, applied
Download (untitled) / with headers
text/plain 155b
... forgot to notice, https://github.com/eserte/perl-tk.git is completely unrelated to Tcl module, it does not use Tcl module. (I think you also know that)
RT-Send-CC: tcltk [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
Hi, I am trying to understand the content of the code disposal patches. So far I cannot fully make it out but I think it would be more efficient than the previous one. There are something I'd like to discuss: 1) The new version put document of originally internal function create_tcl_sub() as a new API. IMHO programmers should always use the form "$interp->call(...,WRITABLE=>$sub)" if they want code disposal to be handled automatically; otherwise, they could just apply original API CreateCommnd() - DeleteCommand() pair to handle code creation and disposal manually. Providing another create_tcl_sub() - _code_dispose() pair looks duplicating and confusing. Besides, user may find it difficult to provide DESCRNAME arg value. 2) The newest POD in github HEAD mentioned $interp->call('fileevent','sock9827430','writable'); won't dispose the sub created from $interp->call('fileevent','sock9827430', 'writable'=>sub{...}); is it still true? Or only 'set' command behaves in this way? 3) The POD mentioned two functions _code_destroy() and destroy_ref() which cannot be found in Tcl.pm. I guess you mean _code_dispose() and delete_ref(). IMO we just need to have delete_ref() in POD. Disclosing both of them might be confusing too. Thanks, SJ
Download (untitled) / with headers
text/plain 558b
I agree with all points. yet, I think that too careful track on coderefs could result in even more memory leaks, not only will grow allocated numbers of CODE references, but also grow hash which tracks them. IMO the best solution to the problem - is documenting it better; if the problem hurts user - she can elaborate another way in her code, change "-command=>sub{}" to "-command=>'tclsub'", where 'tclsub' will handle all the job; maybe warn user in case of 100 allocated coderefs; IDK. we could solve this problem by just ignoring its existence :):)
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Mon, 02 Jul 2018 02:27:49 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 4.4k
At 08:10 PM 7/1/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >Hi, > >I am trying to understand the content of the code disposal patches. >So far I cannot fully make it out but I think it would be more >efficient than the previous one. There are something I'd like to discuss:
What you need to first remember is that i did not introduce code disposal to the module but instead i was trying to find a way to fix problems that the introduction of code disposal had broken in my perl programs that ran fine under v1.02. my first idea with "weaken" did not address all the problems i eventually found as i investigated what code disposal was doing, so i tried to present a more robust solution in my second attempt that addressed all the problems i had discovered. It was never my intent to make my code more efficient that the v1.05 code disposal process, i just wanted to fix it so it no longer broke my existing working code. I added comments to the tests i added to t/ to document the problems i noticed with the v1.05 process that you may find helpful to read. Show quoted text
>1) The new version put document of originally internal function >create_tcl_sub() as a new API. IMHO programmers should always use >the form "$interp->call(...,WRITABLE=>$sub)" if they want code >disposal to be handled automatically; otherwise, they could just >apply original API CreateCommnd() - DeleteCommand() pair to handle >code creation and disposal manually. Providing another >create_tcl_sub() - _code_dispose() pair looks duplicating and >confusing. Besides, user may find it difficult to provide DESCRNAME arg value.
I found that others were already using the create_tcl_sub() entry even though it was not documented. And i found that the description of CreateCommnd() to be confusing to me as well. Since create_tcl_sub() is what was used in call(), and that was that pair where the code needed to be patched i thought it made sense to document how create_tcl_sub() worked and how code disposal worked within it. IMHO it made sense to expose create_tcl_sub() so that programmers could use the automatic recoding of a call(,,$coderef) process as well as being able to control when code disposal took place if they wanted to handle it themselves. This allows them to continue to use coderefs if they want and just delete one use by using DESCRNAME, or all clear it totally by using TCLNAME. I think i made it clear that the user supplied DESCRNAME can be anything they want it to be, as long as it does not begin with '=' or '@', or '::perl::'. '=' now prefaces "normal" entries created by create_tcl_sub(). '@' prefaces entries create_tcl_sub() will use to possibly dispose of coderefs "AT" a future time since they were used by an after command. and '::perl::' prefaces the TCLNAME assigned by create_tcl_sub() and is used in anon_refs{} to determine when no more "users" exist and the coderef can now be safely deleted from the tcl namespace. Show quoted text
>2) The newest POD in github HEAD mentioned > > $interp->call('fileevent','sock9827430','writable'); > >won't dispose the sub created from > > $interp->call('fileevent','sock9827430', 'writable'=>sub{...}); > >is it still true? Or only 'set' command behaves in this way?
yes this is still true, it was true in v1.02 and v1.05 and reverted back to that behavior in v1.11. at v1.06 i had tried to make it so the first call would cause code disposal of the sub created by the second, but that was what broke your "set foo" tests. At the time i introduced that behavior i thought it made sense to try to initiate code disposal in those cases, to keep the internal table and tcl namespace clean, but after spending some time trying to decide if "set foo" was the only place this should be avoided i decided that it was safer to revert to the original processing where code disposal would only be triggered if there was a different coderef in the new call. but not when the coderef was now missing. I wrapped the place where it occurs with a flag so it could be reinstated if desired when it was determined that only "set foo" should be handled in this special manner Show quoted text
>3) The POD mentioned two functions _code_destroy() and destroy_ref() >which cannot be found in Tcl.pm. I guess you mean _code_dispose() >and delete_ref(). IMO we just need to have delete_ref() in POD. >Disclosing both of them might be confusing too.
yes you are right, i misspoke when i wrote those sections of the pod and i will submit corrections to fix that soon. Show quoted text
>Thanks, >SJ
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Mon, 02 Jul 2018 13:59:09 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 8.7k
At 01:51 AM 7/2/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >I agree with all points. > >yet, I think that too careful track on coderefs could result in even >more memory leaks, not only will grow allocated numbers of CODE >references, but also grow hash which tracks them.
I think it would be very hard to have more memory leaks than there were before my patch to tcl.xs because up to that point no coderefs would ever be released at all. Note that this means nothing referenced in a coderefs PAD would get freed either. Show quoted text
>IMO the best solution to the problem - is documenting it better; > >if the problem hurts user - she can elaborate another way in her >code, change "-command=>sub{}" to "-command=>'tclsub'", where >'tclsub' will handle all the job; > >maybe warn user in case of 100 allocated coderefs; > >IDK. we could solve this problem by just ignoring its existence :):)
All this would do is bring us back to the level of v1.02 where there was no code disposal at all, or to the level of v1.05 where code disposal worked on with dynamic coderefs (-command=>sub{...}) but did not often work when static coderefs were used (-command=>$coderef). When i am trying to patch existing code i go into a mode where i try to think like the original programmer thinks. I do not claim to know what you folk do with these kind of callbacks but it became clear to me that the v1.05 code disposal process existed to keep the anon_refs hash cleaner when there were a lot of "after" calls and all the calls used dynamically created coderefs (-command=>sub{...}) my first intent was to make it so code disposal would also work when the after commands were to statically scoped coderefs (-command=>$coderef). But as i dug deeper i realized that the problems with statically scoped coderefs went even deeper. So i tried to present a solution that was robust enough to handle all the problems i had discovered. Even though i felt i had mostly succeeded in that quest i feel that for the most part the code disposal routines will be rarely invoked. In my mind the primary use of callbacks are in the Tkx menu, button and bind arenas. If i make an element with some buttons and later delete that element the buttons go away, but code disposal will not get called on those button callbacks and they will continue to exist in $anon_refs. Even if i recreate that element such that it gets the same tk element name and in the same order, under the v1.05 rules it is very likely that code disposal will have destroyed the internal tcl linkage to the perl code if it was passed as a static coderef. The problem exists because of this line $anon_refs{$rname} = bless [\$sub, $interp], 'Tcl::Code'; in create_tcl_sub(). Since $rname is the same as the previous creation Tcl::Code::DESTROY will get called on the existing slot and removes the tcl linkage to that static coderef even though it is about to assign a new value with the exact same static coderef pointer. The same problem can occur even if the order is not the same. Lets say i have 5 buttons all using static coderefs, do-something-a, do-something-b, do-something-c, about, exit. but then i go and rebuild the element with only 4 buttons, b,c,about,exit. when i add the new button for b, it disposes of the code for button a, and assigns b, but then when i go and assign the new c button, it calls code disposal on the subroutine for b, even though b had just been defined to be button 1. the same goes for the about button, when exit is assigned to button 4 it calls code disposal on the tcl linkage to the subroutine for about because that is what used to be button 4. The exit command is protected tho because code disposal never gets called on the old button 5 because there is no new button 5 overriding it. but the old reference to button 5 still exists in $anon_refs too. The same exists for menus and binds. But under the v1.02 rules since there was no code disposal at all this was not a problem. So my point is that code disposal was only of use when there were a lot of "after" calls with dynamically created coderefs. If the coderefs were statically scoped the v1.05 rules busted the tcl linkage to the code ref. If the element containing the buttons, menus and binds were destroyed in tcl/tk code disposal never got called and the entries still existed in $anon_refs. If the element was recreated with the same name it was likely that any static coderefs would not work under the v1.05 rules. So v1.05 code disposal was only useful in "after" commands that used dynamically created coderefs. My patches fixed the problems with statically scoped coderefs. They also introduced a path where the user could control code disposal better. If i called create_tcl_sub() to "register" my static coderef and captured the TCLNAME returned i could later call _code_dispose() or delete_ref() in that TCLNAME and it would clear the tcl linkage to the coderef and clean $anaon_refs of all related entries without needing to change the rest of my code that passed statically scoped coderefs. if i registered a static coderef to an about or exit procedure it would not need to be destroyed and then recreated over and over each time it was used in a new element. There was another problem that existed under v1.05 related to the fact that only the last code reference was saved if there was more than one code reference in the call() arguments. Lets assume $interp->call('if,'1',$sub1,$sub2); It goes and creates a reference for $sub1, and saves it in $anon_refs{$rname} and @codes. It then goes and creates a reference for $sub2 and assigns it to the same place in $anon_refs that held $sub1. but since sub1 is also in @codes it is safe for now. It passes to the code execution phase and since both still exist in @codes it executes fine. But then @codes is DESTROYED and that destroys the internal tcl reference to the perl code. Now $interp->call('if,'1',$sub1,$sub2); is run again. $sub1 no longer exists and the tcl reference gets recreated and assigned to $anon_refs{$rname}, but this causes the internal tcl reference to $sub2 to be destroyed, It then processes $sub2, and again $sub1 is protected since it is still in @codes, and it executes fine but then when @codes is destroyed the internal tcl reference to $sub1 is removed and only the reference to $sub1 still exists. This delete/recreate cycle will keep repeating but this didnt happen under v1.02 because that version tested to see if the TCLNAME didnt already exist before trying to create it. It was to prevent this that i decided it was best to save all of @codes in $anaon_refs{$rname} for each call(). So now that you understand my goals for the patches it should be clear that i reached them while still retaining the original code disposal goal of cleaning up "after" calls that used dynamically scoped coderefs. But i introduced a bug that was exhibited by the "set foo" test in tkx. This was the last part i introduced and obviously it wasnt not as well thought out as the other goals. I wanted to be able to trigger code disposal when a readable or writable callback was removed. As i am cleaning up my sockets in preparation for disposal of the socket itself clearing those callbacks was the first thing i did, and i figured it would also be done by anyone else using those callbacks. It would also clean up bind calls where there was no coderef/script supplied which serves to delete the current binding for that sequence. just like it does for readable/writable. While i still think that only the "set foo" instance should be dealt with i backed out that whole rule just in case, but left it wrapped in a flag should we want to bring it back for all but the "set foo" instance. IMHO the use of CreateCommand() should be avoided by the user since there is little clear guidance as to what the CLIENTDATA, and DELETEPROC arguments do and how they are handled if they are undef. it took a lot of digging thru tcl.xs to understand that. But It is clear in tcl.pm how create_tcl_sub() is called and used and that serves as a reference to the user about how to call it themselves. She runs more of a risk in using CreateCommand() wrong than using create_tcl_sub() which does most of the background work for you of creating a TCLNAME and DELETEPROC for you, and passes useful CLIENTDATA to that DELETEPROC. And since most of the uses for these kind of callbacks seem to be by Tkx programs i was going to suggest that more documentation be added to Tkx.pm about them, about how the new code disposal routines affect tkx commands and that a route to call create_tcl_sub() and _code_dispose/delete_ref without needing to first find Tkx::i::interp be introduced. IHMO it is in the Tkx arena that the documentation is most lacking about this and needs more enhancement.
Download (untitled) / with headers
text/plain 567b
we very much appreciate the effort; unclear for me - why $sub1 and $sub2 will have same slot for this statement, $interp->call('if,'1',$sub1,$sub2); there is: ... # stringify sub, becomes "CODE(0x######)" in ::perl namespace so $sub1 and $sub2 will recieve different slots in %anon_refs hash all in all, if you'll find a correct solution - nice. if isn't - also not bad. for mee, we already have too much code for the case, which I would prefer to be simplier. also, what a need for sub _anon_refs_cheat { return \%anon_refs;} ? can we switch to mailing list?
RT-Send-CC: tcltk [...] perl.org
Download (untitled) / with headers
text/plain 4.3k
Thank you for the prompt and detailed response. Show quoted text
> I added comments to the tests i added to t/ to document the problems > i noticed with the v1.05 process that you may find helpful to read. > > I found that others were already using the create_tcl_sub() entry > even though it was not documented. And i found that the description > of CreateCommnd() to be confusing to me as well.
I think someone uses the internal function does not necessary mean that the function has to be exposed by document. The difference is: If a module user uses undocumented internal function, he/she takes the risk of function robustness and compatibility on module revision. On the other hand, if a module document a function as an API, the module shall take response that both interface and behavior should not change in the future. Show quoted text
> Since create_tcl_sub() is what was used in call(), and that was that > pair where the code needed to be patched i thought it made sense to > document how create_tcl_sub() worked and how code disposal worked > within it. IMHO it made sense to expose create_tcl_sub() so that > programmers could use the automatic recoding of a call(,,$coderef) > process as well as being able to control when code disposal took > place if they wanted to handle it themselves. This allows them to > continue to use coderefs if they want and just delete one use by > using DESCRNAME, or all clear it totally by using TCLNAME.
Yes, it makes sense to document how internal mechanism works, but again it does not mean create_tcl_sub() has to be documented so detail and as a part of API. Show quoted text
> I think i made it clear that the user supplied DESCRNAME can be > anything they want it to be, as long as it does not begin with '=' > or '@', or '::perl::'. > '=' now prefaces "normal" entries created by create_tcl_sub(). > '@' prefaces entries create_tcl_sub() will use to possibly dispose of > coderefs "AT" a future time since they were used by an after command. > and '::perl::' prefaces the TCLNAME assigned by create_tcl_sub() and > is used in anon_refs{} to determine when no more "users" exist and > the coderef can now be safely deleted from the tcl namespace.
Okay, I finally get the the point of DESCRNAME usage now. I did not know that DESCRNAME can be passed to _code_dispose() so I did not get the purpose of DESCRNAME at all. I suggest to have more explicit description that argument of _code_dispose() is either TCLNAME or DESCRNAME. And the major purpose of DESCRNAME is used to pass to _code_disposal(). I get your point more, but I still think we shall properly define the interface to support code disposal instead of just directly expose an internal function out. For example, how about modify call() to support the form of call(DESCRNAME, cmd, arg ...), where DESCRNAME is optional and is a string that with '@' prefix. Then we have a good compatibility and user do not have to worry about GENCODE, EVENTS etc. Show quoted text
> > 2) The newest POD in github HEAD mentioned > > > > $interp->call('fileevent','sock9827430','writable'); > > > > won't dispose the sub created from > > > > $interp->call('fileevent','sock9827430', 'writable'=>sub{...}); > > > > is it still true? Or only 'set' command behaves in this way?
> > yes this is still true, it was true in v1.02 and v1.05 and reverted > back to that behavior in v1.11. at v1.06 i had tried to make it so > the first call would cause code disposal of the sub created by the > second, but that was what broke your "set foo" tests. At the time i > introduced that behavior i thought it made sense to try to initiate > code disposal in those cases, to keep the internal table and tcl > namespace clean, but after spending some time trying to decide if > "set foo" was the only place this should be avoided i decided that > it was safer to revert to the original processing where code > disposal would only be triggered if there was a different coderef in > the new call. but not when the coderef was now missing. > > I wrapped the place where it occurs with a flag so it could be > reinstated if desired when it was determined that only "set foo" > should be handled in this special manner
Too bad that it is disabled by default. IMO '$interp->call("FILEEVENT" ... "WRITABLE=>"")' shall be a suggested way to dispose the code and well documented. 'set' command is an exception and to be documented. Of course there are other exceptions to be found out. The flag SAVENOCODE might be better documented in POD too. SJ
RT-Send-CC: tcltk [...] perl.org
Download (untitled) / with headers
text/plain 755b
VKON wrote: Show quoted text
> I agree with all points. > > yet, I think that too careful track on coderefs could result in even > more memory leaks, not only will grow allocated numbers of CODE > references, but also grow hash which tracks them. > > IMO the best solution to the problem - is documenting it better; > > if the problem hurts user - she can elaborate another way in her code, > change "-command=>sub{}" to "-command=>'tclsub'", where 'tclsub' will > handle all the job; > > maybe warn user in case of 100 allocated coderefs; > > IDK. we could solve this problem by just ignoring its existence :):)
Yes, I think it's a good idea to get user warned; but I prefer only on 'use warnings;' specified and criterion to be over 1000 allocated coderefs. SJ
CC: tcltk [...] perl.org
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Tue, 03 Jul 2018 12:27:36 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 1.6k
At 03:07 AM 7/3/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >we very much appreciate the effort; > >unclear for me - why $sub1 and $sub2 will have same slot for this statement, >$interp->call('if,'1',$sub1,$sub2); > >there is: >... ># stringify sub, becomes "CODE(0x######)" in ::perl namespace > >so $sub1 and $sub2 will recieve different slots in %anon_refs hash
in the v1.05 code $current_r will be set once at the top of the loop by my $current_r = join ' ', grep {defined} grep {!ref} @args; so it will be 'if 1' and is passed into create_tcl_sub() in the 4th parm where it becomes $rname and $anon_refs gets set with $anon_refs{$rname} = bless [\$sub, $interp], 'Tcl::Code'; so both calls get sequentially created into the same $anon_refs slot $anon_refs{'if 1'} And before the second gets assigned Tcl::Code::DESTROY is triggered on the first. Show quoted text
>all in all, if you'll find a correct solution - nice. if isn't - >also not bad. for mee, we already have too much code for the case, >which I would prefer to be simplier.
This kind of reference tracking is never simple, i have tried to keep it as simple as it needs to be. Show quoted text
>also, what a need for >sub _anon_refs_cheat { return \%anon_refs;} > >?
it serves to expose %anon_refs to external programs so they can test what has been done to it by the process in the module. I often do this to be able to test that things are working right without having to add other exposing code to the primary module. Show quoted text
>can we switch to mailing list?
i think i got subscribed, so this is cc'd to the list. for those first seeing this on the list there is historical background at https://rt.cpan.org/Public/Bug/Display.html?id=125472
CC: tcltk [...] perl.org
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Tue, 03 Jul 2018 14:02:42 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 9.9k
Also cc'd to tcltk@perl.org https://www.nntp.perl.org/group/perl.tcltk/2018/07/msg718.html per Vadim's request At 09:07 AM 7/3/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > > >Thank you for the prompt and detailed response. >
> > I added comments to the tests i added to t/ to document the problems > > i noticed with the v1.05 process that you may find helpful to read. > > > > I found that others were already using the create_tcl_sub() entry > > even though it was not documented. And i found that the description > > of CreateCommnd() to be confusing to me as well.
> >I think someone uses the internal function does not necessary mean >that the function has to be exposed by document. The difference is: >If a module user uses undocumented internal function, he/she takes >the risk of function robustness and compatibility on module >revision. On the other hand, if a module document a function as an >API, the module shall take response that both interface and behavior >should not change in the future.
I agree that documented interfaces should continue to work as documented, but that does not mean they must stay static, it just means they should not bust any pervious uses. After looking at tcl.pm/.xs it was clear to me that create_tcl_sub() should be the preferred method for introducing code to tcl because CreateCommand() was so vaguely documented and its arguments are very specific to the tcl interface and can cause bigger problems is used improperly. create_tcl_sub() takes care of much of the needed background processing needed to properly create a tcl<=>perl subroutine such as placeing it into the proper namespace and returing the tcl name in that namespace. it also populates the tracking structure such that further uses of a static coderef will not repopulate the tcl subroutine internal reference (in v1.02- and v1.06+ at least) if it is found a second time by call(). Show quoted text
> > Since create_tcl_sub() is what was used in call(), and that was that > > pair where the code needed to be patched i thought it made sense to > > document how create_tcl_sub() worked and how code disposal worked > > within it. IMHO it made sense to expose create_tcl_sub() so that > > programmers could use the automatic recoding of a call(,,$coderef) > > process as well as being able to control when code disposal took > > place if they wanted to handle it themselves. This allows them to > > continue to use coderefs if they want and just delete one use by > > using DESCRNAME, or all clear it totally by using TCLNAME.
> >Yes, it makes sense to document how internal mechanism works, but >again it does not mean create_tcl_sub() has to be documented so >detail and as a part of API. >
> > I think i made it clear that the user supplied DESCRNAME can be > > anything they want it to be, as long as it does not begin with '=' > > or '@', or '::perl::'. > > '=' now prefaces "normal" entries created by create_tcl_sub(). > > '@' prefaces entries create_tcl_sub() will use to possibly dispose of > > coderefs "AT" a future time since they were used by an after command. > > and '::perl::' prefaces the TCLNAME assigned by create_tcl_sub() and > > is used in anon_refs{} to determine when no more "users" exist and > > the coderef can now be safely deleted from the tcl namespace.
> >Okay, I finally get the the point of DESCRNAME usage now. I did not >know that DESCRNAME can be passed to _code_dispose() so I did not >get the purpose of DESCRNAME at all. I suggest to have more explicit >description that argument of _code_dispose() is either TCLNAME or >DESCRNAME. And the major purpose of DESCRNAME is used to pass to >_code_disposal(). >I get your point more, but I still think we shall properly define >the interface to support code disposal instead of just directly >expose an internal function out. For example, how about modify >call() to support the form of call(DESCRNAME, cmd, arg ...), where >DESCRNAME is optional and is a string that with '@' prefix. Then we >have a good compatibility and user do not have to worry about >GENCODE, EVENTS etc.
The main purpose of DESCRNAME is to name the slot used in $anon_refs to store information about the tcl<=>perl subroutine linkage and track it. Just one of the purposes of tracking is to be able to dispose of it later. I do not understand why you mean to accomplish by modifying call() that way, how would call understand that it is a descrname and not a tcl verb? The @ prefix was selected to identify a group of coderefs that might be able to be deleted by a call() generated "after" call to _code_dispose once the initiating "after" call has completed. For the most part $anon_refs keys beginning with '@" should be very short lived transients. I think adding another use for the '@' prefix would be counterproductive. And a call() can also have more than one coderef in its args as call('if','1',$sub1,$sub2) illustrates. The documentation for Tcl::_code_destroy(NAME) already states Show quoted text
>calling _code_destroy on a TCLNAME retruned from create_tcl_sub
removes all use instances and purges the command table. Show quoted text
>calling _code_destroy on a DESCRNAME passed to create_tcl_sub
removes only that instace Show quoted text
>Code used in a DESCRNAME may be used in other places as well, >only when the last usage is purged does the entry get purged from
the command table I think that is a pretty good description of its modes, but if you can suggest a better description you are free to have a go at it. I can understand why you think that EVENTS should be hidden better, and then GENCODE as well, but both of them were well integrated into the existing code and i did not want to break that. I can also see cases where a user may want to use the EVENTS parm tho, and in that case they would want both the TCLNAME and GENCODE returned, so i introduced a change to what was returned to allow for that. Existing code expecting a scalar to be returned still gets GENCODE as it did before, but new code expecting an array can get both of them. And uless EVENTS was populated both are the same anyway. Show quoted text
> > > 2) The newest POD in github HEAD mentioned > > > > > > $interp->call('fileevent','sock9827430','writable'); > > > > > > won't dispose the sub created from > > > > > > $interp->call('fileevent','sock9827430', 'writable'=>sub{...}); > > > > > > is it still true? Or only 'set' command behaves in this way?
> > > > yes this is still true, it was true in v1.02 and v1.05 and reverted > > back to that behavior in v1.11. at v1.06 i had tried to make it so > > the first call would cause code disposal of the sub created by the > > second, but that was what broke your "set foo" tests. At the time i > > introduced that behavior i thought it made sense to try to initiate > > code disposal in those cases, to keep the internal table and tcl > > namespace clean, but after spending some time trying to decide if > > "set foo" was the only place this should be avoided i decided that > > it was safer to revert to the original processing where code > > disposal would only be triggered if there was a different coderef in > > the new call. but not when the coderef was now missing. > > > > I wrapped the place where it occurs with a flag so it could be > > reinstated if desired when it was determined that only "set foo" > > should be handled in this special manner
> >Too bad that it is disabled by default. IMO >'$interp->call("FILEEVENT" ... "WRITABLE=>"")' shall be a suggested >way to dispose the code and well documented. 'set' command is an >exception and to be documented. Of course there are other exceptions >to be found out. The flag SAVENOCODE might be better documented in POD too.
i did not document SAVENOCODE nor SAVEALLCODES because i feel both to be transient flags destined to be deleted when the tcl.pm code is solidified. both serve to temporarily encapsulate code i felt to be useful but might not deserve to be in the final code, and allow the encapsulated code to be toggled on/off to see how it works in different situations. If you look at where both are used i think you will find that i did document what i was trying to do when i introduced the code they encapsulate. Again if you think you can explain it better please feel free to do so. i am not a fan of "Of course there are other exceptions to be found out." methods. It disgusted me greatly that my introduction of the code to deal with '$interp->call("FILEEVENT" ... "WRITABLE=>"")' (or its bind cousins) busted the "set foo" test in Tkx.pm. I hate to bust existing working code and feel it diminishes my reputation as a responsible programmer when i do so. I would rather shut it off for now, reverting to the existing process in v1.02/v1.05 than to play catchup to bug reports stating it busted something else. I have been thru much of the tcl/tk documentation looking for other cases the newer behavior may also break but so far have only identified the "set foo" instance, but that does not mean no other cases still exist and would rather revert to the previous behavior until i can be assured that the "set foo" instance is the only one to be exempted rather than bust someone elses code and require them to figure out how to file a bug report about it. When we are assured that the code will do what we want it is a simple matter to remove the encapsulation and delete the transient flags. Show quoted text
>SJ
This new code disposal process is opening a can of worms, but that does not mean it cannot be accomplished successfully. As i was working on the reference counting i imagined inserting a new code disposal method that could clean up after a deleted tk element, searching the keys of %anon_refs for keys prefixed with '=' (internally generated) and tracking button/bind/menu objects related to a element name specified as a parameter. It could then dispose of them in the normal method, freeing any that are no longer referenced. while i did not think i should being this up until the code disposal code has stabilized more, i think now is not a bad time to mention it as it serves to solidify the direction i think code_disposal should proceed in.
CC: tcltk [...] perl.org
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Tue, 03 Jul 2018 14:21:20 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 2.4k
Also cc'd to tcltk@perl.org https://www.nntp.perl.org/group/perl.tcltk/2018/07/msg718.html per Vadim's request At 09:23 AM 7/3/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >VKON wrote: >
> > I agree with all points. > > > > yet, I think that too careful track on coderefs could result in even > > more memory leaks, not only will grow allocated numbers of CODE > > references, but also grow hash which tracks them. > > > > IMO the best solution to the problem - is documenting it better; > > > > if the problem hurts user - she can elaborate another way in her code, > > change "-command=>sub{}" to "-command=>'tclsub'", where 'tclsub' will > > handle all the job; > > > > maybe warn user in case of 100 allocated coderefs; > > > > IDK. we could solve this problem by just ignoring its existence :):)
> >Yes, I think it's a good idea to get user warned; but I prefer only >on 'use warnings;' specified and criterion to be over 1000 allocated coderefs. > >SJ
I think it is important to discriminate between the end user of a tck/tk application and the programmer of that application. What would an end user think if he started getting flooded with warnings for a existing application they use that they cannot control, that only the application developer can modify. I too think that 100 is too small a threshold. I have a tkx application that provides me buttons to run programs via a deamon running on other boxes. There are 50 buttons for just one of the boxes, and that means 50 menu entries as well. There are 6 other boxes that can be selected (the buttons panel and menu gets recreated when switched) and they too have dozens of buttons and menu items. I am working on using code disposal to keep $anon_refs clean in this application but under v1.02.v1.05 rules of no disposal i could easily generate hundreds of entries to $anon_refs just by cycling thru the different boxes. Each time a button/menu is activated it creates a new toplevel window where the output results of the command executed on the other box gets displayed, and each of these toplevel windows has its own menu as well. I used to keep this application open for weeks at a time, but lately only open and close it as needed to reclaim desktop and task bar space. To tell you the truth, under v1.02 i did not see a performance penalty to introducing all these entries to $anon_refs and the tcl<=>perl command table. This leads me to believe that 100 may be way to small and even 1000 might be as well.
CC: "tcltk [...] perl.org" <tcltk [...] perl.org>
Subject: RE: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Fri, 6 Jul 2018 20:39:57 +0000
To: huck <huck [...] finn.com>, "bug-Tcl [...] rt.cpan.org" <bug-Tcl [...] rt.cpan.org>
From: "Konovalov, Vadim" <Vadim.Konovalov [...] dell.com>
Download (untitled) / with headers
text/plain 1.2k
Show quoted text
> Also cc'd to > tcltk@perl.org > https://www.nntp.perl.org/group/perl.tcltk/2018/07/msg718.html > per Vadim's request
Thanks Show quoted text
> I think it is important to discriminate between the end user of a tck/tk > application and the programmer of that application. What would an end user > think if he started getting flooded with warnings for a existing application > they use that they cannot control, that only the application developer can > modify.
Correct; the only good solution is to have correct implementation of refs disposal; BTW perl/Tk happen to have this problem solved; Tcl.pm having some difficulties due to different approach on the problem. Show quoted text
> > I too think that 100 is too small a threshold. I have a > .... > To tell you the > truth, under v1.02 i did not see a performance penalty to introducing all these > entries to $anon_refs and the tcl<=>perl command table. This leads me to > believe that 100 may be way to small and even 1000 might be as well.
Any number will do! :) BTW are you aware that, in terms of performance, this: sub WIDGET_CLEANUP() {0} if (WIDGET_CLEANUP) { ... is much better than this: our $SAVEALLCODES; # ... ... if ($SAVEALLCODES) { ... It's OK for development, but rather significant for release;
RT-Send-CC: tcltk [...] perl.org
Download (untitled) / with headers
text/plain 10.9k
Hi, Show quoted text
> > I think someone uses the internal function does not necessary mean > > that the function has to be exposed by document. The difference is: > > If a module user uses undocumented internal function, he/she takes > > the risk of function robustness and compatibility on module > > revision. On the other hand, if a module document a function as an > > API, the module shall take response that both interface and behavior > > should not change in the future.
> > I agree that documented interfaces should continue to work as > documented, but that does not mean they must stay static, it just > means they should not bust any pervious uses.
Yes. But any change has possibility to bust previous uses. The more unnecessary internal function exposing, the more compatibility issue have to be considered and the more easily to bust existing use. Show quoted text
> After looking at tcl.pm/.xs it was clear to me that create_tcl_sub() > should be the preferred method for introducing code to tcl because > CreateCommand() was so vaguely documented and its arguments are very > specific to the tcl interface and can cause bigger problems is used > improperly. > > create_tcl_sub() takes care of much of the needed background > processing needed to properly create a tcl<=>perl subroutine such as > placeing it into the proper namespace and returing the tcl name in > that namespace. it also populates the tracking structure such that > further uses of a static coderef will not repopulate the tcl > subroutine internal reference (in v1.02- and v1.06+ at least) if it > is found a second time by call().
Well I don't think CreateCommand() document vague: CLIENTDATA is just any user private data to be pass to the callback function. DELETEPROC is served as the destructor function that is to be called when DeleteCommand() applied to the created tcl command. They are optional it's no harm to set them undef if you do not know what it is. Show quoted text
> > > I think i made it clear that the user supplied DESCRNAME can be > > > anything they want it to be, as long as it does not begin with '=' > > > or '@', or '::perl::'. > > > '=' now prefaces "normal" entries created by create_tcl_sub(). > > > '@' prefaces entries create_tcl_sub() will use to possibly dispose > > > of > > > coderefs "AT" a future time since they were used by an after > > > command. > > > and '::perl::' prefaces the TCLNAME assigned by create_tcl_sub() > > > and > > > is used in anon_refs{} to determine when no more "users" exist and > > > the coderef can now be safely deleted from the tcl namespace.
> > > > Okay, I finally get the the point of DESCRNAME usage now. I did not > > know that DESCRNAME can be passed to _code_dispose() so I did not > > get the purpose of DESCRNAME at all. I suggest to have more explicit > > description that argument of _code_dispose() is either TCLNAME or > > DESCRNAME. And the major purpose of DESCRNAME is used to pass to > > _code_disposal(). > > I get your point more, but I still think we shall properly define > > the interface to support code disposal instead of just directly > > expose an internal function out. For example, how about modify > > call() to support the form of call(DESCRNAME, cmd, arg ...), where > > DESCRNAME is optional and is a string that with '@' prefix. Then we > > have a good compatibility and user do not have to worry about > > GENCODE, EVENTS etc.
> > > The main purpose of DESCRNAME is to name the slot used in $anon_refs > to store information about the tcl<=>perl subroutine linkage and > track it. Just one of the purposes of tracking is to be able to > dispose of it later.
Show quoted text
> I do not understand why you mean to accomplish by modifying call() > that way, how would call understand that it is a descrname and not a > tcl verb? > > The @ prefix was selected to identify a group of coderefs that > might be able to be deleted by a call() generated "after" call to > _code_dispose once the initiating "after" call has completed. For > the most part $anon_refs keys beginning with '@" should be very short > lived transients. I think adding another use for the '@' prefix would > be counterproductive. And a call() can also have more than one > coderef in its args as call('if','1',$sub1,$sub2) illustrates.
My idea is to apply a prefix to identify if first argument of call() is a DESCRNAME. If the first arg call() is '%' , then it is a DESCRNAME. (Let's forget previous AT sign '@' example, which might confuse you). Otherwise the first arg is a tcl verb. For example the following code: call('%button1', ..., $sub1, sub{}) call('%button1', ..., $sub2, $sub3) call('if', '1', $sub3) : code_dispose('button1'); Then inline sub{}, $sub1 and $sub2 would get disposed while #sub3 is kept. The code looks much cleaner compared to annoying TCLNAME/GENCODE/EVENT dealing and storing. Here one DESCRNAME maps to multiple $subs, which is not allowed in current implementation. Just share an idea for a better disposal interface. Show quoted text
> The documentation for Tcl::_code_destroy(NAME) already states
> > calling _code_destroy on a TCLNAME retruned from create_tcl_sub
> removes all use instances and purges the command table.
> > calling _code_destroy on a DESCRNAME passed to create_tcl_sub
> removes only that instace
> > Code used in a DESCRNAME may be used in other places as well, > > only when the last usage is purged does the entry get purged from
> the command table > > I think that is a pretty good description of its modes, but if you > can suggest a better description you are free to have a go at it. > > I can understand why you think that EVENTS should be hidden better, > and then GENCODE as well, but both of them were well integrated into > the existing code and i did not want to break that. I can also see > cases where a user may want to use the EVENTS parm tho, and in that > case they would want both the TCLNAME and GENCODE returned, so i > introduced a change to what was returned to allow for that. Existing > code expecting a scalar to be returned still gets GENCODE as it did > before, but new code expecting an array can get both of them. And > uless EVENTS was populated both are the same anyway. >
Yes, I think it is weird to return twin variable TCLNAME/GENCODE. They are mostly the same thing and I need two variables to store them. As a user, I'd like such function just return one thing/object that can be pass to both call() and _code_dispose(). EVENT is also a stuff that should not be worried by user: we are already familiar with Tk::Ev() and it is also weird to provide yet another variable/argument serving the same purpose. Show quoted text
> > > > 2) The newest POD in github HEAD mentioned > > > > > > > > $interp->call('fileevent','sock9827430','writable'); > > > > > > > > won't dispose the sub created from > > > > > > > > $interp->call('fileevent','sock9827430', 'writable'=>sub{...}); > > > > > > > > is it still true? Or only 'set' command behaves in this way?
> > > > > > yes this is still true, it was true in v1.02 and v1.05 and reverted > > > back to that behavior in v1.11. at v1.06 i had tried to make it so > > > the first call would cause code disposal of the sub created by the > > > second, but that was what broke your "set foo" tests. At the time i > > > introduced that behavior i thought it made sense to try to initiate > > > code disposal in those cases, to keep the internal table and tcl > > > namespace clean, but after spending some time trying to decide if > > > "set foo" was the only place this should be avoided i decided that > > > it was safer to revert to the original processing where code > > > disposal would only be triggered if there was a different coderef > > > in > > > the new call. but not when the coderef was now missing. > > > > > > I wrapped the place where it occurs with a flag so it could be > > > reinstated if desired when it was determined that only "set foo" > > > should be handled in this special manner
> > > > Too bad that it is disabled by default. IMO > > '$interp->call("FILEEVENT" ... "WRITABLE=>"")' shall be a suggested > > way to dispose the code and well documented. 'set' command is an > > exception and to be documented. Of course there are other exceptions > > to be found out. The flag SAVENOCODE might be better documented in > > POD too.
> > i did not document SAVENOCODE nor SAVEALLCODES because i feel both to > be transient flags destined to be deleted when the tcl.pm code is > solidified. > > both serve to temporarily encapsulate code i felt to be useful but > might not deserve to be in the final code, and allow the encapsulated > code to be toggled on/off to see how it works in different > situations. If you look at where both are used i think you will find > that i did document what i was trying to do when i introduced the > code they encapsulate. Again if you think you can explain it better > please feel free to do so. > > i am not a fan of "Of course there are other exceptions to be found > out." methods. It disgusted me greatly that my introduction of the > code to deal with '$interp->call("FILEEVENT" ... "WRITABLE=>"")' (or > its bind cousins) busted the "set foo" test in Tkx.pm. I hate to > bust existing working code and feel it diminishes my reputation as a > responsible programmer when i do so. > > I would rather shut it off for now, reverting to the existing process > in v1.02/v1.05 than to play catchup to bug reports stating it busted > something else. I have been thru much of the tcl/tk documentation > looking for other cases the newer behavior may also break but so far > have only identified the "set foo" instance, but that does not mean > no other cases still exist and would rather revert to the previous > behavior until i can be assured that the "set foo" instance is the > only one to be exempted rather than bust someone elses code and > require them to figure out how to file a bug report about it. When > we are assured that the code will do what we want it is a simple > matter to remove the encapsulation and delete the transient flags. > > > This new code disposal process is opening a can of worms, but that > does not mean it cannot be accomplished successfully. As i was > working on the reference counting i imagined inserting a new code > disposal method that could clean up after a deleted tk element, > searching the keys of %anon_refs for keys prefixed with '=' > (internally generated) and tracking button/bind/menu objects related > to a element name specified as a parameter. It could then dispose of > them in the normal method, freeing any that are no longer referenced. > while i did not think i should being this up until the code disposal > code has stabilized more, i think now is not a bad time to mention it > as it serves to solidify the direction i think code_disposal should > proceed in.
BTW, I found _code_dispose() has to be called in the form 'Tcl::_code_dispose(...)' rather than '$interp->_code_dispose(...)'. It is documented, but it is counter-intuitive. Programmers can hardly find their incorrect calling via $interp object. I suggest to check input parameter and provide some warning at least. IMO, %anon_refs{} would be better an object instance variable instead of global one. SJ
CC: tcltk [...] perl.org
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Sat, 07 Jul 2018 09:46:24 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 17.7k

Message body is not shown because it is too large.

CC: tcltk [...] perl.org
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Sat, 14 Jul 2018 09:49:55 +0800
To: bug-Tcl [...] rt.cpan.org
From: "S.J. Luo" <sjaluo [...] gmail.com>
Download (untitled) / with headers
text/plain 5.7k
2018-07-07 22:46 GMT+08:00 huck : Show quoted text
> > Yes, as we saw when code disposal was introduced , even existing > applications can be ruined if care is not properly taken. Maybe if > create_tcl_sub() had been documented as part of the api this problem would > not have been created in the first place. >
I think it had not been because create_tcl_sub() was created for internal use only and not intended to be an API. Show quoted text
>
>> >> My idea is to apply a prefix to identify if first argument of call() is a >> DESCRNAME. If the first arg call() is '%' , then it is a DESCRNAME. (Let's >> forget previous AT sign '@' example, which might confuse you). Otherwise the >> first arg is a tcl verb. For example the following code: >> >> call('%button1', ..., $sub1, sub{}) >> call('%button1', ..., $sub2, $sub3) >> call('if', '1', $sub3) >> : >> code_dispose('button1'); >> >> Then inline sub{}, $sub1 and $sub2 would get disposed while #sub3 is kept. >> The code looks much cleaner compared to annoying TCLNAME/GENCODE/EVENT >> dealing and storing. Here one DESCRNAME maps to multiple $subs, which is not >> allowed in current implementation. Just share an idea for a better disposal >> interface.
> > > > And how would you implement this via Tkx? > for instance how do you modify a $menu>add_command(%args); call? > or $button =$in->new_ttk__button(-text=>$args{text}, -command=>$args{sub} ); >
It's not for Tkx user. It's for Tcl user (such as Tkx). Tkx can associate a unique DESCRNAME 'menu1' with an object $menu and call code_dispose('menu1') on destroyer of $button1. In previous example, $sub1, inline sub{}, $sub2 get disposed when code_dispose('button1') is called. They get disposed because they are associated with DESCRNAME 'button1' via call('%button1'). $sub3 is also associated with 'button1', it does not get disposed because it has also been call()'ed once without DESCRNAME. Show quoted text
> like you said before if you dont know what to do with EVENT it can be > defined as undef, and then TCLNAME is the same as GENCODE (as the doc says) > and only that one item needs to be saved, just like you need to save the > CMDNAME passed to CreateCommand () > > Changing the existing and working interface to call() seems to be much more > error prone than documenting the interface to create_tcl_sub(), just making > things more complex. > > As i already mentioned by using create_tcl_sub() to manage code disposal, i > can just add one call to create_tcl_sub() to lock down a sub from code > disposal, and add another call to _code_destroy() when i want to release > that coderef. Your proposed method requires every call with a coderef to be > modified which was one thing i was trying to avoid. > > And you say "would get disposed", but when? the only time code may get > automagicly disposed is when something has the same DESCRNAME, but here you > propose allowing DESCRNAME ot have the same value over multiple calls, are > you proposing that inside the second call $sub1 and sub{}) get disposed? > or are you proposing that $sub3 gets added to that list for disposal later? > > And a DESCRNAME can have more than one sub attached to it since v1.06 What > it is is that when the same DESCRNAME is seen again, the prior code calls > are now eligible for code destruction. One change i made is to delay code > destuction untill after the entire command is parsed, so as not to have to > destroy/recreate a coderef that already exists in both the current and the > prior call with the same DESCRNAME This wasnt a problem at v1.02 because > %anon_refs was only assigned to if the TCLNAME did not exist already >
Show quoted text
>> BTW, I found _code_dispose() has to be called in the form >> 'Tcl::_code_dispose(...)' rather than '$interp->_code_dispose(...)'. It is >> documented, but it is counter-intuitive. Programmers can hardly find their >> incorrect calling via $interp object. I suggest to check input parameter and >> provide some warning at least. IMO, %anon_refs{} would be better an object >> instance variable instead of global one.
> > > > but it isnt an object instance now, as far as i can tell never has been, So > the change you are proposing here is not trivial. What should it be an > instance of? > > While $interp can be considered an object all it is really is a blessed > scalar. So where do you propose you store this new object instance? If we > make $interp be the first key to %anon_refs that will cause a lot of code to > need to be changed. > > And as i pointed out while delete_ref() is called with an $interp for the > most part that doesnt matter, since the $interp stored in the Tcl::Code > object is used for the deletion, and while there could be many different > interpreters, there is only one %anon_refs, so that on a single TCLNAME can > exist, there cannot be one for each interpreter.
I mean %anon_refs shall be better to be something like $interp->{anon_refs}. Different interpreters shall be better have separated TCLNAME/CMDNAME/DESCRNAME naming space. It does not make sense that TCLNAME/DESCRNAME have to be unique across different interpreters. Show quoted text
> Again, maybe the solution is to make a RFP asking for input on code > disposal, what it should do, how it should be called and what modification > will be needed to existing code. Without that my intentions were to make it > so existing v1.02 code still worked, and there was a mechanism to control > code disposal. I think my patches have met both of those goals. (and as i > watched code disposal in action i realized it was destroying/recreating tcl > subs over and over and i wanted to prevent that too) > > and i also realized that a deep bug in the xs code was leading to a massive > memory leak and fixed that as well.
Yes I believe v1.06 got quite some bugs fixed. Here just some discussion for any possibility to make interface more clean. SJ
CC: tcltk [...] perl.org
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Fri, 13 Jul 2018 23:43:32 -0500
To: bug-Tcl [...] rt.cpan.org
From: huck <huck [...] finn.com>
Download (untitled) / with headers
text/plain 10.8k
At 08:50 PM 7/13/2018, you wrote: Show quoted text
><URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > > >2018-07-07 22:46 GMT+08:00 huck : >
> > > > Yes, as we saw when code disposal was introduced , even existing > > applications can be ruined if care is not properly taken. Maybe if > > create_tcl_sub() had been documented as part of the api this problem would > > not have been created in the first place. > >
> >I think it had not been because create_tcl_sub() was created for >internal use only and not intended to be an API.
Maybe if it had been documented the original code disposal changes would not have busted existing programs like they did. I also find it hard to believe that just because something was one designed for internal use it means it can never be exposed as part of the API. create_tcl_sub() is now integral to the code disposal changes. It is the place that code disposal is now controled. allowing external access to it is basically the only way to allow the developer to control code disposal. Show quoted text
> >
> >> > >> My idea is to apply a prefix to identify if first argument of call() is a > >> DESCRNAME. If the first arg call() is '%' , then it is a DESCRNAME. (Let's > >> forget previous AT sign '@' example, which might confuse you).
> Otherwise the
> >> first arg is a tcl verb. For example the following code: > >> > >> call('%button1', ..., $sub1, sub{}) > >> call('%button1', ..., $sub2, $sub3) > >> call('if', '1', $sub3) > >> : > >> code_dispose('button1'); > >> > >> Then inline sub{}, $sub1 and $sub2 would get disposed while #sub3 is kept. > >> The code looks much cleaner compared to annoying TCLNAME/GENCODE/EVENT > >> dealing and storing. Here one DESCRNAME maps to multiple $subs,
> which is not
> >> allowed in current implementation. Just share an idea for a
> better disposal
> >> interface.
> > > > > > > > And how would you implement this via Tkx? > > for instance how do you modify a $menu>add_command(%args); call? > > or $button =$in->new_ttk__button(-text=>$args{text},
> -command=>$args{sub} );
> >
> > It's not for Tkx user. It's for Tcl user (such as Tkx). Tkx can >associate a unique DESCRNAME 'menu1' with an object $menu and call >code_dispose('menu1') on destroyer of $button1.
Ya know i dont care much about the Tcl only user. I use it from Tkx, somebody busted tkx. i fixed it. QED. Who is going to make the changes to Tkx to do this? How do i pass this information from my program to tkx to do it? Right now tkx is a simple intercept interface between perl and ttk. Basically a tkx developer does not interface with call() at all. How do you propose they do it under your scenario? My WORKING solution allows the tkx user to get access to code disposal via create_tcl_sub(). I dont hear any solutions from you , just proposals. And what destroyer of $button1 are you talking about? can you show me where Tkx sets up a destroyer for a button? Or are you proposing that someone come up with the new code? Are you going to do it? tkx just recodes a perl call into the corresponding tk name and calls that tk procedure. Show quoted text
> In previous example, $sub1, inline sub{}, $sub2 get disposed when >code_dispose('button1') is called. They get disposed because they are >associated with DESCRNAME 'button1' via call('%button1'). $sub3 is >also associated with 'button1', it does not get disposed because it >has also been call()'ed once without DESCRNAME.
It is never called without a DESCRNAME, call() always assigns a DESCRNAME when it calls create_tcl_sub(), i think your proposal to add a leading argument to call() has not been well thought out yet. The arguments to call() are very well defined now, they constitute the elements of a call to tcl, now you want to majorly modify what the arguments are for no good reason i can figure out so far. Show quoted text
> > like you said before if you dont know what to do with EVENT it can be > > defined as undef, and then TCLNAME is the same as GENCODE (as the doc says) > > and only that one item needs to be saved, just like you need to save the > > CMDNAME passed to CreateCommand () > > > > Changing the existing and working interface to call() seems to be much more > > error prone than documenting the interface to create_tcl_sub(), just making > > things more complex. > > > > As i already mentioned by using create_tcl_sub() to manage code disposal, i > > can just add one call to create_tcl_sub() to lock down a sub from code > > disposal, and add another call to _code_destroy() when i want to release > > that coderef. Your proposed method requires every call with a
> coderef to be
> > modified which was one thing i was trying to avoid. > > > > And you say "would get disposed", but when? the only time code may get > > automagicly disposed is when something has the same DESCRNAME, but here you > > propose allowing DESCRNAME ot have the same value over multiple calls, are > > you proposing that inside the second call $sub1 and sub{}) get disposed? > > or are you proposing that $sub3 gets added to that list for disposal later? > > > > And a DESCRNAME can have more than one sub attached to it since v1.06 What > > it is is that when the same DESCRNAME is seen again, the prior code calls > > are now eligible for code destruction. One change i made is to delay code > > destuction untill after the entire command is parsed, so as not to have to > > destroy/recreate a coderef that already exists in both the current and the > > prior call with the same DESCRNAME This wasnt a problem at v1.02 because > > %anon_refs was only assigned to if the TCLNAME did not exist already > >
>
> >> BTW, I found _code_dispose() has to be called in the form > >> 'Tcl::_code_dispose(...)' rather than '$interp->_code_dispose(...)'. It is > >> documented, but it is counter-intuitive. Programmers can hardly find their > >> incorrect calling via $interp object. I suggest to check input
> parameter and
> >> provide some warning at least. IMO, %anon_refs{} would be
> better an object
> >> instance variable instead of global one.
> > > > > > > > but it isnt an object instance now, as far as i can tell never has been, So > > the change you are proposing here is not trivial. What should it be an > > instance of? > > > > While $interp can be considered an object all it is really is a blessed > > scalar. So where do you propose you store this new object instance? If we > > make $interp be the first key to %anon_refs that will cause a lot
> of code to
> > need to be changed. > > > > And as i pointed out while delete_ref() is called with an $interp for the > > most part that doesnt matter, since the $interp stored in the Tcl::Code > > object is used for the deletion, and while there could be many different > > interpreters, there is only one %anon_refs, so that on a single TCLNAME can > > exist, there cannot be one for each interpreter.
> > I mean %anon_refs shall be better to be something like >$interp->{anon_refs}. Different interpreters shall be better have >separated TCLNAME/CMDNAME/DESCRNAME naming space. It does not make >sense that TCLNAME/DESCRNAME have to be unique across different >interpreters.
It is not that way now, and as far as i can tell it was never that way ever. What you are asking for is not a simple change to make,. it changes a lot of things. my $inter=Tcl->new(); print Dumper($inter); $VAR1 = bless( do{\(my $o = 149405600)}, 'Tcl' ); there is no place for $interp->{anon_refs} to live. it is just a blessed scalar. Have you even looked at the code at all yet? Show quoted text
> > Again, maybe the solution is to make a RFP asking for input on code > > disposal, what it should do, how it should be called and what modification > > will be needed to existing code. Without that my intentions were to make it > > so existing v1.02 code still worked, and there was a mechanism to control > > code disposal. I think my patches have met both of those goals. (and as i > > watched code disposal in action i realized it was destroying/recreating tcl > > subs over and over and i wanted to prevent that too) > > > > and i also realized that a deep bug in the xs code was leading to a massive > > memory leak and fixed that as well.
> >Yes I believe v1.06 got quite some bugs fixed. Here just some >discussion for any possibility to make interface more clean.
Yes my fixes have made v1.11 working again. All is working at least like it used to and there is elementary code disposal for 'after' calls like intorduced after v1.02. All i see is trying to make the interface more complicated, without proposing any code changes yourself. You want to change what $inter is, from a blessed scalar to a blessed hash. you want to change how call() is defined, yoiu are proposing major changes to tkx.pm that will complicate it by introducing some sort of interface between all the perl and tcl calls that will create destroyers. So are you proposing to pay me to do all this or are you going to do it yourself? I think the easiest thing here is for you to make your own branch and put in the changes you want, show me the working code that passes all the tests. Then explain how it is cleaner, and why it is any better than what we have now. I have no desire to do something just because you want it done. I wanted to get my tkx programs working again, i put the time in to make it so, and i think i did a much better job than what you have proposed so far. It you can do better than go right ahead. My proposal for you is to start with a statement of what you want code disposal to do. then describe how you will modify tcl.pm to do that, then describe how you will modify tkx.pm to incorporate that path. Once you have reached that point publish your proposal and see what comments you get back. Then you can go in and study v1.02 and v1.05 and see what changed, once you have reached that point you can start making the improvements to the code. Then you can publish a patch so we can install it to a local branch and see how it works. Again, my purpose when I took on this task to make tcl better than it was because for no fault of mine my programs stopped working. I spent a lot of time looking thru the code and learning how to think like the authors did when they wrote their code. The change from v1.02 to v1.05 was not minor, It changed a lot of how %anon_refs was used. My fix incorporates the original design of %anon_refs tracking the TCLNAME so it doenst constantly redefine perl <->tcl coderefs, it also tracks the new DESCRNAME used for code disposal purposes, I does not redefine a TCLNAME that currently exists, and only destroys it when all the registered users of that TCLNAME have been eliminated. I joined the original v1.02 and the v1.05 code in a way that satisfies the purposes of both of them It requires no changes to existing tcl/tkx programs or to tkx.pm to work properly. It also allows for developer access to this code disposal procedure via create_tcl_sub() so they can have a say in how code disposal works for them. Show quoted text
>SJ
Subject: Re: [rt.cpan.org #125472] error with coderefs passes as scalar args
Date: Sat, 14 Jul 2018 22:11:35 +0800
To: bug-Tcl [...] rt.cpan.org, tcltk [...] perl.org
From: "S.J. Luo" <sjaluo [...] gmail.com>
Download (untitled) / with headers
text/plain 11.4k
Okay I agree that showing some code would make the point more convincing and I have plan to try it. Let's stop the discussion. SJ 2018-07-14 12:43 GMT+08:00 huck <huck@finn.com>: Show quoted text
> At 08:50 PM 7/13/2018, you wrote:
>> >> <URL: https://rt.cpan.org/Ticket/Display.html?id=125472 > >> >> 2018-07-07 22:46 GMT+08:00 huck : >>
>> > >> > Yes, as we saw when code disposal was introduced , even existing >> > applications can be ruined if care is not properly taken. Maybe if >> > create_tcl_sub() had been documented as part of the api this problem >> > would >> > not have been created in the first place. >> >
>> >> I think it had not been because create_tcl_sub() was created for >> internal use only and not intended to be an API.
> > > Maybe if it had been documented the original code disposal changes would not > have busted existing programs like they did. > > I also find it hard to believe that just because something was one designed > for internal use it means it can never be exposed as part of the API. > > create_tcl_sub() is now integral to the code disposal changes. It is the > place that code disposal is now controled. allowing external access to it is > basically the only way to allow the developer to control code disposal. > > >
>> >
>> >> >> >> My idea is to apply a prefix to identify if first argument of call() is >> >> a >> >> DESCRNAME. If the first arg call() is '%' , then it is a DESCRNAME. >> >> (Let's >> >> forget previous AT sign '@' example, which might confuse you). >> >> Otherwise the >> >> first arg is a tcl verb. For example the following code: >> >> >> >> call('%button1', ..., $sub1, sub{}) >> >> call('%button1', ..., $sub2, $sub3) >> >> call('if', '1', $sub3) >> >> : >> >> code_dispose('button1'); >> >> >> >> Then inline sub{}, $sub1 and $sub2 would get disposed while #sub3 is >> >> kept. >> >> The code looks much cleaner compared to annoying TCLNAME/GENCODE/EVENT >> >> dealing and storing. Here one DESCRNAME maps to multiple $subs, which >> >> is not >> >> allowed in current implementation. Just share an idea for a better >> >> disposal >> >> interface.
>> > >> > >> > >> > And how would you implement this via Tkx? >> > for instance how do you modify a $menu>add_command(%args); call? >> > or $button =$in->new_ttk__button(-text=>$args{text}, >> > -command=>$args{sub} ); >> >
>> >> It's not for Tkx user. It's for Tcl user (such as Tkx). Tkx can >> associate a unique DESCRNAME 'menu1' with an object $menu and call >> code_dispose('menu1') on destroyer of $button1.
> > > Ya know i dont care much about the Tcl only user. I use it from Tkx, > somebody busted tkx. i fixed it. QED. > > Who is going to make the changes to Tkx to do this? How do i pass this > information from my program to tkx to do it? Right now tkx is a simple > intercept interface between perl and ttk. Basically a tkx developer does > not interface with call() at all. How do you propose they do it under your > scenario? > > My WORKING solution allows the tkx user to get access to code disposal via > create_tcl_sub(). I dont hear any solutions from you , just proposals. > > And what destroyer of $button1 are you talking about? can you show me where > Tkx sets up a destroyer for a button? Or are you proposing that someone come > up with the new code? Are you going to do it? > > tkx just recodes a perl call into the corresponding tk name and calls that > tk procedure. >
>> In previous example, $sub1, inline sub{}, $sub2 get disposed when >> code_dispose('button1') is called. They get disposed because they are >> associated with DESCRNAME 'button1' via call('%button1'). $sub3 is >> also associated with 'button1', it does not get disposed because it >> has also been call()'ed once without DESCRNAME.
> > > > It is never called without a DESCRNAME, call() always assigns a DESCRNAME > when it calls create_tcl_sub(), i think your proposal to add a leading > argument to call() has not been well thought out yet. The arguments to > call() are very well defined now, they constitute the elements of a call to > tcl, now you want to majorly modify what the arguments are for no good > reason i can figure out so far. > > >
>> > like you said before if you dont know what to do with EVENT it can be >> > defined as undef, and then TCLNAME is the same as GENCODE (as the doc >> > says) >> > and only that one item needs to be saved, just like you need to save the >> > CMDNAME passed to CreateCommand () >> > >> > Changing the existing and working interface to call() seems to be much >> > more >> > error prone than documenting the interface to create_tcl_sub(), just >> > making >> > things more complex. >> > >> > As i already mentioned by using create_tcl_sub() to manage code >> > disposal, i >> > can just add one call to create_tcl_sub() to lock down a sub from code >> > disposal, and add another call to _code_destroy() when i want to release >> > that coderef. Your proposed method requires every call with a coderef >> > to be >> > modified which was one thing i was trying to avoid. >> > >> > And you say "would get disposed", but when? the only time code may get >> > automagicly disposed is when something has the same DESCRNAME, but here >> > you >> > propose allowing DESCRNAME ot have the same value over multiple calls, >> > are >> > you proposing that inside the second call $sub1 and sub{}) get >> > disposed? >> > or are you proposing that $sub3 gets added to that list for disposal >> > later? >> > >> > And a DESCRNAME can have more than one sub attached to it since v1.06 >> > What >> > it is is that when the same DESCRNAME is seen again, the prior code >> > calls >> > are now eligible for code destruction. One change i made is to delay >> > code >> > destuction untill after the entire command is parsed, so as not to have >> > to >> > destroy/recreate a coderef that already exists in both the current and >> > the >> > prior call with the same DESCRNAME This wasnt a problem at v1.02 because >> > %anon_refs was only assigned to if the TCLNAME did not exist already >> >
>>
>> >> BTW, I found _code_dispose() has to be called in the form >> >> 'Tcl::_code_dispose(...)' rather than '$interp->_code_dispose(...)'. It >> >> is >> >> documented, but it is counter-intuitive. Programmers can hardly find >> >> their >> >> incorrect calling via $interp object. I suggest to check input >> >> parameter and >> >> provide some warning at least. IMO, %anon_refs{} would be better an >> >> object >> >> instance variable instead of global one.
>> > >> > >> > >> > but it isnt an object instance now, as far as i can tell never has been, >> > So >> > the change you are proposing here is not trivial. What should it be an >> > instance of? >> > >> > While $interp can be considered an object all it is really is a blessed >> > scalar. So where do you propose you store this new object instance? If >> > we >> > make $interp be the first key to %anon_refs that will cause a lot of >> > code to >> > need to be changed. >> > >> > And as i pointed out while delete_ref() is called with an $interp for >> > the >> > most part that doesnt matter, since the $interp stored in the Tcl::Code >> > object is used for the deletion, and while there could be many different >> > interpreters, there is only one %anon_refs, so that on a single TCLNAME >> > can >> > exist, there cannot be one for each interpreter.
>> >> I mean %anon_refs shall be better to be something like >> $interp->{anon_refs}. Different interpreters shall be better have >> separated TCLNAME/CMDNAME/DESCRNAME naming space. It does not make >> sense that TCLNAME/DESCRNAME have to be unique across different >> interpreters.
> > > It is not that way now, and as far as i can tell it was never that way ever. > What you are asking for is not a simple change to make,. it changes a lot of > things. > > my $inter=Tcl->new(); print Dumper($inter); > $VAR1 = bless( do{\(my $o = 149405600)}, 'Tcl' ); > > there is no place for $interp->{anon_refs} to live. it is just a blessed > scalar. > > Have you even looked at the code at all yet? > >
>> > Again, maybe the solution is to make a RFP asking for input on code >> > disposal, what it should do, how it should be called and what >> > modification >> > will be needed to existing code. Without that my intentions were to make >> > it >> > so existing v1.02 code still worked, and there was a mechanism to >> > control >> > code disposal. I think my patches have met both of those goals. (and as >> > i >> > watched code disposal in action i realized it was destroying/recreating >> > tcl >> > subs over and over and i wanted to prevent that too) >> > >> > and i also realized that a deep bug in the xs code was leading to a >> > massive >> > memory leak and fixed that as well.
>> >> Yes I believe v1.06 got quite some bugs fixed. Here just some >> discussion for any possibility to make interface more clean.
> > > Yes my fixes have made v1.11 working again. All is working at least like it > used to and there is elementary code disposal for 'after' calls like > intorduced after v1.02. > > All i see is trying to make the interface more complicated, without > proposing any code changes yourself. You want to change what $inter is, from > a blessed scalar to a blessed hash. you want to change how call() is > defined, yoiu are proposing major changes to tkx.pm that will complicate it > by introducing some sort of interface between all the perl and tcl calls > that will create destroyers. > > So are you proposing to pay me to do all this or are you going to do it > yourself? > > I think the easiest thing here is for you to make your own branch and put in > the changes you want, show me the working code that passes all the tests. > Then explain how it is cleaner, and why it is any better than what we have > now. I have no desire to do something just because you want it done. > > I wanted to get my tkx programs working again, i put the time in to make it > so, and i think i did a much better job than what you have proposed so far. > It you can do better than go right ahead. > > My proposal for you is to start with a statement of what you want code > disposal to do. then describe how you will modify tcl.pm to do that, then > describe how you will modify tkx.pm to incorporate that path. Once you have > reached that point publish your proposal and see what comments you get back. > > Then you can go in and study v1.02 and v1.05 and see what changed, once you > have reached that point you can start making the improvements to the code. > Then you can publish a patch so we can install it to a local branch and see > how it works. > > Again, my purpose when I took on this task to make tcl better than it was > because for no fault of mine my programs stopped working. I spent a lot of > time looking thru the code and learning how to think like the authors did > when they wrote their code. The change from v1.02 to v1.05 was not minor, > It changed a lot of how %anon_refs was used. My fix incorporates the > original design of %anon_refs tracking the TCLNAME so it doenst constantly > redefine perl <->tcl coderefs, it also tracks the new DESCRNAME used for > code disposal purposes, I does not redefine a TCLNAME that currently exists, > and only destroys it when all the registered users of that TCLNAME have been > eliminated. I joined the original v1.02 and the v1.05 code in a way that > satisfies the purposes of both of them > > It requires no changes to existing tcl/tkx programs or to tkx.pm to work > properly. > > It also allows for developer access to this code disposal procedure via > create_tcl_sub() so they can have a say in how code disposal works for them. >
>> SJ


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.