TclVFS: View Ticket

Ticket Hash:	60cacfbcb91a4baf0e2416e6d941026e61ebb4b4
Title:	Segmentation Fault at interpreter exit when tclvfs loaded
Status:	Closed	Type:	Code_Defect
Severity:	Critical	Priority:	Immediate
Subsystem:		Resolution:	Fixed
Last Modified:	2023-09-05 11:25:47 3.12 years ago	Created:	2023-09-04 18:31:57 3.12 years ago
Version Found In:	Any

User Comments:

anonymous added on 2023-09-04 18:31:57:

Tested with tcl 8.7 - crash occurs on Windows 10 as well as FreeBSD 13 Not all builds susceptible - some from Bawt and my own were.

simplest test script: interp create a a eval {package require vfs} interp delete a

from running with gdb I found a loop between the Vfs_UnregisterWithInterp callback and it's call to Tcl_deleteAssocData

#3  0x00007ff9290e159e in Vfs_UnregisterWithInterp (dummy=0x1, interp=0x26623f8) at generic/vfs.c:431
431             res = Vfs_RemoveMount(NULL, interp);

#4  0x00007ff8df80dfd7 in Tcl_DeleteAssocData (interp=0x26623f8,name=0x7ff9290e8029 "vfs::inUse") at C:/buildtcl/core8d/tcl/generic/tclBasic.c:1674
1674            dPtr->proc(dPtr->clientData, interp);

#5  0x00007ff9290e15c9 in Vfs_UnregisterWithInterp (dummy=0x1, interp=0x26623f8) at generic/vfs.c:434
434         Tcl_DeleteAssocData(interp, "vfs::inUse");

#6  0x00007ff8df80dfd7 in Tcl_DeleteAssocData (interp=0x26623f8, name=0x7ff9290e8029 "vfs::inUse") at C:/buildtcl/core-8d/tcl/generic/tclBasic.c:1674
1674            dPtr->proc(dPtr->clientData, interp);

I believe the fix is not to call Tcl_DeleteAssocData from within the callback. Instead Tcl_SetAssocData can be used to unset the AssocData by using NULL arguments

e.g I made the following adjustment in Vfs_UnregisterWithInterp and the problem went away.

/* Make sure our assoc data has been deleted - do not use Tcl_deleteAssocData - as that triggers this callback*/ Tcl_SetAssocData(interp, "vfs::inUse", NULL, NULL); /* Tcl_DeleteAssocData(interp, "vfs::inUse"); */

anonymous added on 2023-09-05 06:44:42:

I believe the real culprit is the implementation of Tcl_DeleteAssocData()
which looks up the string key, if it is found, performs the delete callback,
and finally deletes the AssocData record.

Would the last two steps be done in reverse, there would not be a potential
recursion, ever.

Maybe this ticket should be cross linked on Tcl's repo, too.

anonymous added on 2023-09-05 06:48:05:

I.e. in code

--- orig ---

    hPtr = Tcl_FindHashEntry(iPtr->assocData, name);
    if (hPtr == NULL) {
        return;
    }
    dPtr = (AssocData *) Tcl_GetHashValue(hPtr);
    if (dPtr->proc != NULL) {
        dPtr->proc(dPtr->clientData, interp);
    }
    ckfree(dPtr);
    Tcl_DeleteHashEntry(hPtr);

--- new ---

    hPtr = Tcl_FindHashEntry(iPtr->assocData, name);
    if (hPtr == NULL) {
        return;
    }
    dPtr = (AssocData *) Tcl_GetHashValue(hPtr);
    Tcl_DeleteHashEntry(hPtr);
    if (dPtr->proc != NULL) {
        dPtr->proc(dPtr->clientData, interp);
    }
    ckfree(dPtr);

jan.nijtmans added on 2023-09-05 11:19:24:

Fixed here in Tcl Core

jan.nijtmans added on 2023-09-05 11:25:47:

Additional fix [f082c47f9b2e0456|here]. Since Tcl already takes care of removing the AssocData, it shouldn't be done in the callback