Tcl Source Code

The following script:

proc deleter {ns args} {
    namespace delete $ns
}

namespace eval n1 {
    proc c1 {} {}
}
trace add command n1::c1 delete [list deleter ::n1]
proc ::n1::c1 {} {}

produces the error:

called Tcl_CreateHashEntry on deleted table
Aborted (core dumped)

Closing again since as of [50d71c14f4e0210c] valgrind reports no memory errors.

The fix for [ba1419303b4c] probably partially addressed the issues described here.

Updated status:

8.5 remains broken with valgrind complaining.

8.6 has minimal workaround patch silencing valgrind, but
the foundations are still rotten.

8.7 gets 8.6 treatment for now, but should become the
arena to work out the right solution to these matters.

dgp	line 3141 wants cmdPtr->nsPtr to not be freed.
   * pooryorick opens core-8-6-branch...
dgp	the Namespace struct does have a refCount field
<pooryorick>	line 3141, which file?
dgp	tclBasic.c
<pooryorick>	In Tcl_DeleteCommandFromToken?
dgp	heh, you've already fixed this on 8.7 ? 
<pooryorick>	Is was thinking maybe I had.
<pooryorick>	But not in that function...
<pooryorick>	Sounds like it's just about fixed here.
dgp	next thing is to look at the similar patterns
<pooryorick>	In core-8-branch there's something similar in TclReadeObjCommandInNs, which I might have added.
<pooryorick>	But it's still unguarded in Tcl_DeleteCommandFromToken.
dgp	line 1810 is similar, also broken, but not tested?
<pooryorick>	1810 is empty in my checkout, but it's in Tcl_HideCommand
<pooryorick>	I don't see anything in there that would decrement the namespace refCount.
dgp	we have delete and rename traces, but not hide traces, I guess
dgp	Namespace refCounts are not used in nearly as many places as I would have guessed.
dgp	Certainly not by everything that stores an nsPtr.
dgp	Only parent namespaces and ResolvedNsName intreps appear to use it.

The memory management brokenness of this work needs fixing for 8.6.8 release.

Re-opening this ticket.

The new test basic-15.2 produces invalid reads.

Whether the fix introduced this, or the new
test just exposes it, a proper fix should fix
that as well.

==6980== Invalid read of size 4
==6980==    at 0x423DAA: Tcl_DeleteCommandFromToken (tclBasic.c:2898)
==6980==    by 0x422EA7: Tcl_CreateObjCommand (tclBasic.c:2124)
==6980==    by 0x4E52BB: Tcl_ProcObjCmd (tclProc.c:186)
==6980==    by 0x4251F1: TclEvalObjvInternal (tclBasic.c:3776)
==6980==    by 0x48B855: TclExecuteByteCode (tclExecute.c:2412)
==6980==    by 0x489936: TclCompEvalObj (tclExecute.c:1533)
==6980==    by 0x42778F: TclEvalObjEx (tclBasic.c:5345)
==6980==    by 0x4E6476: Tcl_UplevelObjCmd (tclProc.c:945)
==6980==  Address 0xc69ee10 is 416 bytes inside a block of size 440 free'd
==6980==    at 0x4C2ACDD: free (vg_replace_malloc.c:530)
==6980==    by 0x52EE93: TclpFree (tclAlloc.c:723)
==6980==    by 0x42D5E3: Tcl_Free (tclCkalloc.c:1221)
==6980==    by 0x4C938A: NamespaceFree (tclNamesp.c:1237)
==6980==    by 0x4C9088: Tcl_DeleteNamespace (tclNamesp.c:1042)
==6980==    by 0x4CC09D: NamespaceDeleteCmd (tclNamesp.c:3220)
==6980==    by 0x4CB499: Tcl_NamespaceObjCmd (tclNamesp.c:2845)
==6980==    by 0x4251F1: TclEvalObjvInternal (tclBasic.c:3776)
==6980==  Block was alloc'd at
==6980==    at 0x4C29BE3: malloc (vg_replace_malloc.c:299)
==6980==    by 0x52EE79: TclpAlloc (tclAlloc.c:700)
==6980==    by 0x42D3E2: Tcl_Alloc (tclCkalloc.c:1058)
==6980==    by 0x4C8BBA: Tcl_CreateNamespace (tclNamesp.c:827)
==6980==    by 0x4CC175: NamespaceEvalCmd (tclNamesp.c:3284)
==6980==    by 0x4CB4D7: Tcl_NamespaceObjCmd (tclNamesp.c:2851)
==6980==    by 0x4251F1: TclEvalObjvInternal (tclBasic.c:3776)
==6980==    by 0x48B855: TclExecuteByteCode (tclExecute.c:2412)

==6980== Invalid read of size 4
==6980==    at 0x423D81: Tcl_DeleteCommandFromToken (tclBasic.c:2898)
==6980==    by 0x422EA7: Tcl_CreateObjCommand (tclBasic.c:2124)
==6980==    by 0x4E52BB: Tcl_ProcObjCmd (tclProc.c:186)
==6980==    by 0x4251F1: TclEvalObjvInternal (tclBasic.c:3776)
==6980==    by 0x48B855: TclExecuteByteCode (tclExecute.c:2412)
==6980==    by 0x489936: TclCompEvalObj (tclExecute.c:1533)
==6980==    by 0x42778F: TclEvalObjEx (tclBasic.c:5345)
==6980==    by 0x4E6476: Tcl_UplevelObjCmd (tclProc.c:945)
==6980==  Address 0xc69edd0 is 352 bytes inside a block of size 440 free'd
==6980==    at 0x4C2ACDD: free (vg_replace_malloc.c:530)
==6980==    by 0x52EE93: TclpFree (tclAlloc.c:723)
==6980==    by 0x42D5E3: Tcl_Free (tclCkalloc.c:1221)
==6980==    by 0x4C938A: NamespaceFree (tclNamesp.c:1237)
==6980==    by 0x4C9088: Tcl_DeleteNamespace (tclNamesp.c:1042)
==6980==    by 0x4CC09D: NamespaceDeleteCmd (tclNamesp.c:3220)
==6980==    by 0x4CB499: Tcl_NamespaceObjCmd (tclNamesp.c:2845)
==6980==    by 0x4251F1: TclEvalObjvInternal (tclBasic.c:3776)
==6980==  Block was alloc'd at
==6980==    at 0x4C29BE3: malloc (vg_replace_malloc.c:299)
==6980==    by 0x52EE79: TclpAlloc (tclAlloc.c:700)
==6980==    by 0x42D3E2: Tcl_Alloc (tclCkalloc.c:1058)
==6980==    by 0x4C8BBA: Tcl_CreateNamespace (tclNamesp.c:827)
==6980==    by 0x4CC175: NamespaceEvalCmd (tclNamesp.c:3284)
==6980==    by 0x4CB4D7: Tcl_NamespaceObjCmd (tclNamesp.c:2851)
==6980==    by 0x4251F1: TclEvalObjvInternal (tclBasic.c:3776)
==6980==    by 0x48B855: TclExecuteByteCode (tclExecute.c:2412)

Fixed.

See branch bug-0e4d88b650 for first draft fix of this
problem in Tcl_CreateObjCommand() on core-8-5-branch.

Needs cleanup and tests, then forward merging.

err ..

https://core.tcl.tk/tcl/artifact/45e6cf14bb353bfa?ln=2300

the call to Tcl_DeleteCommandFromToken() invalidates nsPtr, of course, not
whatever I linked above.

following this with MEM_DEBUG, nsPtr is being invalidated at:

https://core.tcl.tk/tcl/artifact/45e6cf14bb353bfa?ln=2306

looking at Tcl_DeleteNamespace, I think the fix will require use of nsPtr->refCount and understand the NS_DEAD flag.