Tcl Extension Architecture (TEA) Sample Extension

View Ticket
Login
Ticket UUID: 19630c0c49e243fd69ef5f246231d107c78e3909
Title: Add unload procedure to allow deletion of the sample extension dll
Type: RFE Version:
Submitter: oehhar Created on: 2024-12-09 12:52:07
Subsystem: 70. Sample Extension Assigned To: nobody
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2024-12-12 08:37:48
Resolution: Wont Fix Closed By: oehhar
    Closed on: 2024-12-12 08:37:48
Description:

The sample extension has no Sample_Unload procedure and thus, may not be unloaded.

User Comments: oehhar added on 2024-12-12 08:37:10:

It was made clear that the sample extension only shows the minimum. This would add complexity.

Message by Ashok on the core list on this:

Am 12.12.2024 um 09:11 schrieb [email protected]:
The unload command is fraught with too many perils imho to get right in the general case and also of limited value.

I would not clutter the sample extension with either of these but of course your opinion may differ.

/Ashok

Branch and ticket closed. Thank you all, Harald


oehhar added on 2024-12-10 17:21:23:

Emiliano has pointed to interpreter Assoc data.

So, here is the proposed change: [eba8935a06].

I copy some of the information given by Emiliano here:

You get (at least!) three "levels" for storing cliendata to Tcl:

  • Per command, using the clientdata argument of Tcl_CreateObjCommand().
  • Per interp, using Tcl_{Set|Get}AssocData().
  • Per thread, using Tcl_GetThreadData().

In this case, Tcl_GetAssocData is to be used, since you want to wipe the command out from the current interpreter, leaving other interps alone. A simple example (untested !!!)

#define MY_PKG_KEY "My great package key"
Tcl_InterpDeleteProc pkgInterpDeleted; /* called when the interp is deleted */
struct CmdClientData {
    Tcl_Command sha1CmdToken;
};

Sample_Init:
    struct CmdClientData *cmdClientDataPtr;
    cmdClientDataPtr = ckalloc(sizeof(struct CmdClientData));
    Tcl_SetAssocData(interp, MY_PKG_KEY, pkgInterpDeleted, cmdClientDataPtr); 
    cmdClientDataPtr->sha1CmdToken = Tcl_CreateObjCommand(
        interp, "sha1", (Tcl_ObjCmdProc *)Sha1_Cmd,
        cmdClientDataPtr, Sha1_CmdDeleteProc);

Sample_Unload:
    struct CmdClientData *cmdClientDataPtr = (struct CmdClientData *)
        Tcl_GetAssocData(interp, MY_PKG_KEY, NULL);
    /* check whether the pointer is not NULL */
    Tcl_DeleteCommandFromToken(interp, cmdClientDataPtr->sha1CmdToken);
    ckfree(cmdClientDataPtr);
    Tcl_DeleteAssocData(interp, MY_PKG_KEY);

Of course you have to juggle the interaction of Sample_Unload, Sha1CmdDeleteProc and pkgInterpDeleted, since they can be called when:

  • [sha1] cmd is deleted (Sha1_CmdDeleteProc is called).
  • interp is destroyed (pkgInterpDeleted is called).
  • [unload] is called on the external library (Sample_Unload is called).


oehhar added on 2024-12-10 09:14:54:

Ok, checkin [2c5e0e025e] adds thread safety for the sha1 command.

And the command tolkens are also saved in a struct.

But how to get the command tolken struct ot the unload procedure ?

Thanks for any hint, Harald


oehhar added on 2024-12-09 18:02:56:
To late, I am already totally discourraged.
It is sooooo complicated...

Thanks, I may look into this back in a year when I understand at least 5%...

The token must be stored in "clientdata". Something, I never understood....

Thanks,
Harald

jan.nijtmans added on 2024-12-09 17:41:26:

Well, I don't want to discourage you, but ... what if someone does a "rename sha1 somethingelse"? Then the Tcl_DeleteCommand() wont work. Solution: remember the token when the command was created, and remove that.


oehhar added on 2024-12-09 17:21:18:

Yes, for multi interpreter/threads, see ticket [ecf13be4c9].

I tried to remove the commands by commit [37a2611f73].

Thanks for the information.

Learning each day...

Harald


jan.nijtmans added on 2024-12-09 16:44:35:

This won't be sufficient. All memory occupied by the extensions should be released, so the "sha1" and the "::sample::build-info" commands must be removed from the interpreter. Otherwise, those commands can still be called, which leads to a crash (and not only because sha1Contexts and ctxtotalRead are NULL).

B.T.W.: sha1Contexts and ctxtotalRead being global static variables is a bad idea too...... Accessing the "sha1" command from multiple interpreters (or - worse - from multiple threads) will give surprises!


oehhar added on 2024-12-09 12:57:39:

Proposal in commit [d1f80795ac] within branch [19630c0c-unload]