Tcl Source Code

While investigating [fe4d7305a8], which has been fixed, I found another bug.

::history is a proc that calls the namespace ensemble ::tcl::history whose components are procs ::tcl::Hist*.  All these commands are defined in file library/history.tcl, are listed (except ::tcl::history) in library/tclIndex, and are loaded on demand by the autoloader.

File library/history.tcl also sets a trace on command ::history, so that if it is deleted, all the related commands ::tcl::* are deleted, and the array ::tcl::history that stores the command history is unset.

If ::history is renamed or deleted, the autoloader reloads it when the next command is entered into the interactive interpreter.

The user should be able to redefine proc ::history, to provide an alternative to the Tcl built-in command.  However, the attempt may cause the interpreter to cease functioning, notably when the new proc is identical to the old.  The interpreter enters a tight CPU-consuming loop, and never reaches the point where it can accept another command.

What seems to be happening is that the tailcall in ::history attempts to evaluate "history" in namespace ::tcl, expecting to evaluate ::tcl::history, but because that command is absent it evaluates ::history instead, leading to infinite recursion.  A fix is:

1. To list ::tcl::history in library/tclIndex

2. To request ::tcl::history explicitly in the tailcall

The fix is in the attached patch, which is listed below.

Other matters:

(a) Does the trace on ::history (deleting all dependencies ::tcl::*) serve any useful purpose, apart from saving memory?  I suggest removing it.

(b) The interpreter usually catches infinite recursion.  In this case it does not.

In view of the importance of ::history, I would appreciate review before committing this change.  If nobody objects I will also remove the trace mentioned above.


diff -Naur original/library/history.tcl patched/library/history.tcl
--- original/library/history.tcl        2024-01-18 17:18:18.000000000 +0000
+++ patched/library/history.tcl 2024-02-03 16:53:40.144116588 +0000
@@ -53,7 +53,7 @@
     }
 
     # Tricky stuff needed to make stack and errors come out right!
-    tailcall apply {arglist {tailcall history {*}$arglist} ::tcl} $args
+    tailcall apply {arglist {tailcall ::tcl::history {*}$arglist} ::tcl} $args
 }
 

 # (unnamed) --
diff -Naur original/library/tclIndex patched/library/tclIndex
--- original/library/tclIndex   2024-01-18 17:18:18.000000000 +0000
+++ patched/library/tclIndex    2024-02-03 16:45:46.591373179 +0000
@@ -20,6 +20,7 @@
 set auto_index(::auto_mkindex_parser::commandInit) [list source [file join $dir auto.tcl]]
 set auto_index(::auto_mkindex_parser::fullname) [list source [file join $dir auto.tcl]]
 set auto_index(history) [list source [file join $dir history.tcl]]
+set auto_index(::tcl::history) [list source [file join $dir history.tcl]]
 set auto_index(::tcl::HistAdd) [list source [file join $dir history.tcl]]
 set auto_index(::tcl::HistKeep) [list source [file join $dir history.tcl]]
 set auto_index(::tcl::HistClear) [list source [file join $dir history.tcl]]

Fixed [3fe9d2e41913336b|here]. Thanks for the thorough analysis, and the patch!

> (b) A separate issue really ....

Yes, a separate issue. I have no idea whether the interpreter should/could catch this.

(a) OK, let's keep the trace, it was just a bit confusing when trying to understand the bug!

(b) A separate issue really.  Is it expected that the interpreter does not catch the infinite recursion in this case?

> If nobody objects I will also remove the trace mentioned above.

It looks like this trace was added on purpose: https://core.tcl-lang.org/tcl/info/3135d5681f5949c8

Therefore, I think removing it is a bad idea (but better ask @dkf, as he did this commit)

Patch looks OK to me. Committed to a branch now.