Tk Source Code

View Ticket
Login
☰Core Home Timeline Branches Tags Tickets Download Wiki
Attach Edit New Ticket Plaintext
Ticket UUID: 14a9b62e1db54af41b4a39a3741885faf25c672c
Title: crash when closing toplevel window with menus
Type: Bug Version: 8.7a6
Submitter: emiliano Created on: 2023-08-18 15:39:56
Subsystem: 10. Generic Menus Assigned To: sbron
Priority: 5 Medium Severity: Critical
Status: Closed Last Modified: 2023-09-04 15:25:00
Resolution: Fixed Closed By: jan.nijtmans
    Closed on: 2023-09-04 15:25:00
Description: 
$ cat menucrash.tcl 
package require Tk
toplevel .t1 -menu .t1.mbar
set m [menu .t1.mbar]
$m add cascade -label "menu" -menu $m.c
menu $m.c
$m.c add command -label "close" -command [list destroy .t1]

upon invoking the "close " menu

$ tclsh8.7 menucrash.tcl 
alloc: invalid block: 0x11abf40: 20 0
Aborted (core dumped)

gdb backtrace
(gdb) bt
#0  0xb7fd7bb5 in __kernel_vsyscall ()
#1  0xb7bc1a02 in __libc_signal_restore_set (set=0xbfffe5ac)
    at ../sysdeps/unix/sysv/linux/nptl-signals.h:80
#2  __GI_raise (sig=6) at ../sysdeps/unix/sysv/linux/raise.c:48
#3  0xb7bc2e91 in __GI_abort () at abort.c:79
#4  0xb7eef26c in Tcl_PanicVA (
    format=0xb7f906b8 "alloc: invalid block: %p: %x %x", argList=0xbfffe894 "")
    at /home/carla/src/tcl/generic/tclPanic.c:125
#5  0xb7eef2a6 in Tcl_Panic (
    format=0xb7f906b8 "alloc: invalid block: %p: %x %x")
    at /home/carla/src/tcl/generic/tclPanic.c:160
#6  0xb7f19ee4 in Ptr2Block (ptr=0x733c28)
    at /home/carla/src/tcl/generic/tclThreadAlloc.c:832
#7  0xb7f1955c in TclpFree (ptr=0x733c28)
    at /home/carla/src/tcl/generic/tclThreadAlloc.c:398
#8  0xb7eaad29 in Tcl_DeleteHashEntry (entryPtr=0x733c28)
    at /home/carla/src/tcl/generic/tclHash.c:440
#9  0xb69d8250 in DestroyMenuEntry (memPtr=0x7c1508)
    at /home/carla/src/tk/unix/../generic/tkMenu.c:1485
#10 0xb7efe09e in Tcl_Release (clientData=0x7c1508)
    at /home/carla/src/tcl/generic/tclPreserve.c:228
#11 0xb69d7abc in TkInvokeMenu (interp=0x40dde8, menuPtr=0x7a2fc8, index=0)
    at /home/carla/src/tk/unix/../generic/tkMenu.c:1092
#12 0xb69d7222 in MenuWidgetObjCmd (clientData=0x7a2fc8, interp=0x40dde8, 
    objc=3, objv=0x628a6c) at /home/carla/src/tk/unix/../generic/tkMenu.c:889
#13 0xb7dbeaac in Dispatch (data=0x72ac44, interp=0x40dde8, dummy4958=0)
    at /home/carla/src/tcl/generic/tclBasic.c:4996
#14 0xb7dbeb2b in TclNRRunCallbacks (interp=0x40dde8, result=0, rootPtr=0x0)
    at /home/carla/src/tcl/generic/tclBasic.c:5036
#15 0xb7dbe3ea in Tcl_EvalObjv (interp=0x40dde8, objc=3, objv=0x418bf8, 
    flags=2097168) at /home/carla/src/tcl/generic/tclBasic.c:4755
#16 0xb7dc0840 in TclEvalEx (interp=0x40dde8, 
    script=0xbfffee34 "\n   tk::MenuInvoke .t1.#t1#mbar.#t1#mbar#c 1\n", 
    numBytes=45, flags=131072, line=2, clNextOuter=0x0, 
    outerScript=0xbfffee34 "\n   tk::MenuInvoke .t1.#t1#mbar.#t1#mbar#c 1\n")
    at /home/carla/src/tcl/generic/tclBasic.c:5912
#17 0xb7dbfd27 in Tcl_EvalEx (interp=0x40dde8, 
    script=0xbfffee34 "\n   tk::MenuInvoke .t1.#t1#mbar.#t1#mbar#c 1\n", 
    numBytes=45, flags=131072) at /home/carla/src/tcl/generic/tclBasic.c:5576
#18 0xb697fd63 in Tk_BindEvent (bindPtr=0x51bb48, eventPtr=0x709ab0, 
    tkwin=0x626998, numObjects=4, objArr=0xbfffef6c)
    at /home/carla/src/tk/unix/../generic/tkBind.c:2624
#19 0xb698a822 in TkBindEventProc (winPtr=0x626998, eventPtr=0x709ab0)
    at /home/carla/src/tk/unix/../generic/tkCmds.c:320
#20 0xb69969e1 in Tk_HandleEvent (eventPtr=0x709ab0)
    at /home/carla/src/tk/unix/../generic/tkEvent.c:1307
#21 0xb699705e in WindowEventProc (evPtr=0x709aa8, flags=-3)
    at /home/carla/src/tk/unix/../generic/tkEvent.c:1738
#22 0xb7ee9447 in Tcl_ServiceEvent (flags=-3)
    at /home/carla/src/tcl/generic/tclNotify.c:710
#23 0xb7ee972f in Tcl_DoOneEvent (flags=-3)
    at /home/carla/src/tcl/generic/tclNotify.c:943
#24 0xb699766a in Tk_MainLoop ()
    at /home/carla/src/tk/unix/../generic/tkEvent.c:2124
#25 0xb7ee1f18 in Tcl_MainEx (argc=-1, argv=0xbffff274, 
    appInitProc=0x40085a <Tcl_AppInit>, interp=0x40dde8)
    at /home/carla/src/tcl/generic/tclMain.c:592
#26 0x00400847 in main (argc=2, argv=0xbffff274)
    at /home/carla/src/tcl/unix/tclAppInit.c:94


The pointed out line

#9  0xb69d8250 in DestroyMenuEntry (memPtr=0x7c1508)
    at /home/carla/src/tk/unix/../generic/tkMenu.c:1485

comes from commit https://core.tcl-lang.org/tk/info/e2b00bfd293c89ea
User Comments: jan.nijtmans added on 2023-09-04 15:25:00:

Fixed [85d17152d854339b|here]. Many thanks!

nab added on 2023-09-04 11:32:39: 
Hi,
this works for me on macOS Aqua.

thanks,
++
sbron added on 2023-09-04 10:36:13:

I have added test menu-6.17 to Tk 8.6 for this issue and merged it to 8.7.

I then created a bug-14a9b62e1d branch with the proposed fix.

emiliano added on 2023-08-20 15:48:32: 
As Christopher well pointed out, the triggering script can be shortened to

package require Tk
menu .m -tearoff 0
.m add command -command {destroy .m}
.m invoke 0
chrstphrchvz added on 2023-08-19 14:37:02:

Making sure that mePtr->entryPtr is NULL rather than stale is indeed a better fix.

The crash can also be triggered by appending $m.c invoke end to the example code. So I would suggest adding a test for this to menu.test in 8.6 (since the example uses nothing specific to 8.7).

sbron added on 2023-08-19 10:03:56:

Then I guess at least the hash entries need to be cleaned up in the loop in DestroyMenuInstance() that used to destroy the menu entries: 

Index: generic/tkMenu.c
==================================================================
--- generic/tkMenu.c
+++ generic/tkMenu.c
@@ -1197,10 +1197,21 @@
      * Free up all the stuff that requires special handling, then let
      * Tk_FreeConfigOptions handle all the standard option-related stuff.
      */
 
     for (i = menuPtr->numEntries; --i >= 0; ) {
+        /*
+         * Clean up the hash entry for the menu item ID.
+         * This cannot be done when the entry is eventually freed, because
+         * the hash table may already have been deleted by then.
+         */
+
+        if (menuPtr->entries[i]->entryPtr) {
+            Tcl_DeleteHashEntry(menuPtr->entries[i]->entryPtr);
+            menuPtr->entries[i]->entryPtr = NULL;
+        }
+
        /*
         * As each menu entry is deleted from the end of the array of entries,
         * decrement menuPtr->numEntries. Otherwise, the act of deleting menu
         * entry i will dereference freed memory attempting to queue a redraw
         * for menu entries (i+1)...numEntries.

sbron added on 2023-08-19 09:06:43:

Indeed, reverting [499c0467b3] (as a test) makes the crash disappear.

chrstphrchvz added on 2023-08-19 08:37:57:

Tcl ticket b6257b8283 does not appear to involve Tk, so I think it is unrelated to this ticket.

The issue here is an interaction between the recently merged patch [499c0467b3] and the TIP 658 implementation which results in a use-after-free. Previously, DestroyMenuInstance() would call DestroyMenuEntry() and in turn Tcl_DeleteHashEntry() immediately for each entry, and before Tcl_DeleteHashTable() is called. But now Tcl_EventuallyFree() is called instead, and because this is occurring during MenuInvokeEntry() which has called Tcl_Preserve() on the invoked entry, DestroyMenuEntry() and in turn Tcl_DeleteHashEntry() are not called for the invoked entry until MenuInvokeEntry() calls Tcl_Release(), after the hash table was deleted. As reported by AddressSanitizer (-DPURIFY -fsanitize=address): 

==11516==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000751158 at pc 0x000113e05572 bp 0x7ff7b1400ea0 sp 0x7ff7b1400e98
READ of size 8 at 0x604000751158 thread T0
    #0 0x113e05571 in Tcl_DeleteHashEntry tclHash.c:374
    #1 0x11054f2b5 in DestroyMenuEntry tkMenu.c:1485
    #2 0x1140d528e in Tcl_Release tclPreserve.c:228
    #3 0x11053605e in TkInvokeMenu tkMenu.c:1092
    #4 0x110dd881a in -[TKMenu(TKMenuActions) tkMenuItemInvoke:] tkMacOSXMenu.c:370
    #5 0x7ff81648542b in -[NSApplication(NSResponder) sendAction:to:from:]+0x142 (AppKit:x86_64+0x23042b)
    #6 0x7ff81657506b in -[NSMenuItem _corePerformAction]+0x19c (AppKit:x86_64+0x32006b)
    #7 0x7ff816574d78 in -[NSCarbonMenuImpl performActionWithHighlightingForItemAtIndex:]+0x8d (AppKit:x86_64+0x31fd78)
    #8 0x7ff8165bd112 in -[NSMenu performActionForItemAtIndex:]+0x70 (AppKit:x86_64+0x368112)
    #9 0x7ff8165bd098 in -[NSMenu _internalPerformActionForItemAtIndex:]+0x51 (AppKit:x86_64+0x368098)
    #10 0x7ff8165bcee0 in -[NSCarbonMenuImpl _carbonCommandProcessEvent:handlerCallRef:]+0x64 (AppKit:x86_64+0x367ee0)
    #11 0x7ff816559a83 in NSSLMMenuEventHandler+0x3dd (AppKit:x86_64+0x304a83)
    #12 0x7ff81cc51d3e in DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*)+0x564 (HIToolbox:x86_64+0x8d3e)
    #13 0x7ff81cc51187 in SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*)+0x14c (HIToolbox:x86_64+0x8187)
    #14 0x7ff81cc65ce4 in SendEventToEventTarget+0x26 (HIToolbox:x86_64+0x1cce4)
    #15 0x7ff81ccc195a in SendHICommandEvent(unsigned int, HICommand const*, unsigned int, unsigned int, unsigned char, void const*, OpaqueEventTargetRef*, OpaqueEventTargetRef*, OpaqueEventRef**)+0x168 (HIToolbox:x86_64+0x7895a)
    #16 0x7ff81cce5d4c in SendMenuCommandWithContextAndModifiers+0x2d (HIToolbox:x86_64+0x9cd4c)
    #17 0x7ff81cce5cf0 in SendMenuItemSelectedEvent+0x157 (HIToolbox:x86_64+0x9ccf0)
    #18 0x7ff81cce5b38 in FinishMenuSelection(SelectionData*, MenuResult*, MenuResult*)+0x5f (HIToolbox:x86_64+0x9cb38)
    #19 0x7ff81cce654b in MenuSelectCore(MenuData*, Point, double, unsigned int, OpaqueMenuRef**, unsigned short*)+0x270 (HIToolbox:x86_64+0x9d54b)
    #20 0x7ff81cce6240 in _HandleMenuSelection2+0x1ca (HIToolbox:x86_64+0x9d240)
    #21 0x7ff816421534 in _NSHandleCarbonMenuEvent+0xd6 (AppKit:x86_64+0x1cc534)
    #22 0x7ff8164213a1 in _DPSEventHandledByCarbon+0x35 (AppKit:x86_64+0x1cc3a1)
    #23 0x7ff8162927ae in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]+0xb66 (AppKit:x86_64+0x3d7ae)
    #24 0x110e1986e in TkMacOSXEventsCheckProc tkMacOSXNotify.c:560
    #25 0x11402ea27 in Tcl_DoOneEvent tclNotify.c:999
    #26 0x1102838af in Tk_MainLoop tkEvent.c:2124
    #27 0x110354688 in Tk_MainEx tkMain.c:388
    #28 0x10eb02bc2 in main tkAppInit.c:96
    #29 0x7ff812dc341e in start+0x76e (dyld:x86_64+0xfffffffffff6e41e)

0x604000751158 is located 8 bytes inside of 40-byte region [0x604000751150,0x604000751178)
freed by thread T0 here:
    #0 0x10f032889 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0xdd889)
    #1 0x1132eea74 in TclpFree tclAlloc.c:723
    #2 0x113e070c9 in Tcl_DeleteHashTable tclHash.c:465
    #3 0x11053987a in DestroyMenuInstance tkMenu.c:1217
    #4 0x110536c53 in TkDestroyMenu tkMenu.c:1301
    #5 0x110575bbb in TkMenuEventProc tkMenuDraw.c:778
    #6 0x1102770a9 in Tk_HandleEvent tkEvent.c:1285
    #7 0x11041f82c in Tk_DestroyWindow tkWindow.c:1498
    #8 0x110536c40 in TkDestroyMenu tkMenu.c:1296
    #9 0x110575bbb in TkMenuEventProc tkMenuDraw.c:778
    #10 0x1102770a9 in Tk_HandleEvent tkEvent.c:1285
    #11 0x11041f82c in Tk_DestroyWindow tkWindow.c:1498
    #12 0x11041e868 in Tk_DestroyWindow tkWindow.c:1439
    #13 0x11041e868 in Tk_DestroyWindow tkWindow.c:1439
    #14 0x11020329b in Tk_DestroyObjCmd tkCmds.c:501
    #15 0x11338af43 in Dispatch tclBasic.c:4629
    #16 0x11336b5ca in TclNRRunCallbacks tclBasic.c:4645
    #17 0x11337f8e7 in TclEvalObjEx tclBasic.c:6105
    #18 0x11337f759 in Tcl_EvalObjEx tclBasic.c:6086
    #19 0x110535db5 in TkInvokeMenu tkMenu.c:1089
    #20 0x110dd881a in -[TKMenu(TKMenuActions) tkMenuItemInvoke:] tkMacOSXMenu.c:370
    #21 0x7ff81648542b in -[NSApplication(NSResponder) sendAction:to:from:]+0x142 (AppKit:x86_64+0x23042b)
    #22 0x7ff81657506b in -[NSMenuItem _corePerformAction]+0x19c (AppKit:x86_64+0x32006b)
    #23 0x7ff816574d78 in -[NSCarbonMenuImpl performActionWithHighlightingForItemAtIndex:]+0x8d (AppKit:x86_64+0x31fd78)
    #24 0x7ff8165bd112 in -[NSMenu performActionForItemAtIndex:]+0x70 (AppKit:x86_64+0x368112)
    #25 0x7ff8165bd098 in -[NSMenu _internalPerformActionForItemAtIndex:]+0x51 (AppKit:x86_64+0x368098)
    #26 0x7ff8165bcee0 in -[NSCarbonMenuImpl _carbonCommandProcessEvent:handlerCallRef:]+0x64 (AppKit:x86_64+0x367ee0)
    #27 0x7ff816559a83 in NSSLMMenuEventHandler+0x3dd (AppKit:x86_64+0x304a83)
    #28 0x7ff81cc51d3e in DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*)+0x564 (HIToolbox:x86_64+0x8d3e)
    #29 0x7ff81cc51187 in SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*)+0x14c (HIToolbox:x86_64+0x8187)
    #30 0x7ff81cc65ce4 in SendEventToEventTarget+0x26 (HIToolbox:x86_64+0x1cce4)
    #31 0x7ff81ccc195a in SendHICommandEvent(unsigned int, HICommand const*, unsigned int, unsigned int, unsigned char, void const*, OpaqueEventTargetRef*, OpaqueEventTargetRef*, OpaqueEventRef**)+0x168 (HIToolbox:x86_64+0x7895a)
    #32 0x7ff81cce5d4c in SendMenuCommandWithContextAndModifiers+0x2d (HIToolbox:x86_64+0x9cd4c)
    #33 0x7ff81cce5cf0 in SendMenuItemSelectedEvent+0x157 (HIToolbox:x86_64+0x9ccf0)
    #34 0x7ff81cce5b38 in FinishMenuSelection(SelectionData*, MenuResult*, MenuResult*)+0x5f (HIToolbox:x86_64+0x9cb38)
    #35 0x7ff81cce654b in MenuSelectCore(MenuData*, Point, double, unsigned int, OpaqueMenuRef**, unsigned short*)+0x270 (HIToolbox:x86_64+0x9d54b)
    #36 0x7ff81cce6240 in _HandleMenuSelection2+0x1ca (HIToolbox:x86_64+0x9d240)
    #37 0x7ff816421534 in _NSHandleCarbonMenuEvent+0xd6 (AppKit:x86_64+0x1cc534)
    #38 0x7ff8164213a1 in _DPSEventHandledByCarbon+0x35 (AppKit:x86_64+0x1cc3a1)
    #39 0x7ff8162927ae in -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]+0xb66 (AppKit:x86_64+0x3d7ae)
    #40 0x110e1986e in TkMacOSXEventsCheckProc tkMacOSXNotify.c:560
    #41 0x11402ea27 in Tcl_DoOneEvent tclNotify.c:999
    #42 0x1102838af in Tk_MainLoop tkEvent.c:2124
    #43 0x110354688 in Tk_MainEx tkMain.c:388
    #44 0x10eb02bc2 in main tkAppInit.c:96
    #45 0x7ff812dc341e in start+0x76e (dyld:x86_64+0xfffffffffff6e41e)

previously allocated by thread T0 here:
    #0 0x10f032740 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0xdd740)
    #1 0x1132eea54 in TclpAlloc tclAlloc.c:699
    #2 0x1133ed224 in Tcl_Alloc tclCkalloc.c:1038
    #3 0x113e023ac in AllocStringEntry tclHash.c:767
    #4 0x113e04ae9 in CreateHashEntry tclHash.c:322
    #5 0x1105412d7 in MenuAddOrInsert tkMenu.c:2477
    #6 0x11051c42e in MenuWidgetObjCmd tkMenu.c:659
    #7 0x11338af43 in Dispatch tclBasic.c:4629
    #8 0x11336b5ca in TclNRRunCallbacks tclBasic.c:4645
    #9 0x113369acd in Tcl_EvalObjv tclBasic.c:4388
    #10 0x113377e1b in TclEvalEx tclBasic.c:5484
    #11 0x113f59017 in Tcl_FSEvalFileEx tclIOUtil.c:1783
    #12 0x11035368e in Tk_MainEx tkMain.c:340
    #13 0x10eb02bc2 in main tkAppInit.c:96
    #14 0x7ff812dc341e in start+0x76e (dyld:x86_64+0xfffffffffff6e41e)


SUMMARY: AddressSanitizer: heap-use-after-free tclHash.c:374 in Tcl_DeleteHashEntry

I currently do not think [499c0467b3] should be reverted. This one line change avoids the crash, although maybe it is not the best solution either: 

--- generic/tkMenu.c
+++ generic/tkMenu.c
@@ -1481,7 +1481,7 @@ DestroyMenuEntry(
                TCL_GLOBAL_ONLY|TCL_TRACE_WRITES|TCL_TRACE_UNSETS,
                MenuVarProc, mePtr);
     }
-    if (mePtr->entryPtr) {
+    if (mePtr->entryPtr && (menuPtr->numEntries != 0)) {
         Tcl_DeleteHashEntry(mePtr->entryPtr);
         mePtr->entryPtr = NULL;
     }

fvogel added on 2023-08-18 17:49:08:

Possible reference: https://core.tcl-lang.org/tcl/tktview/b6257b82833b69673486ae253be7550489642e42

fvogel added on 2023-08-18 16:07:54: 
Thanks Schelte! I have added your login to the list of possible assignees and have assigned this ticket to you.
sbron added on 2023-08-18 15:55:18:

As author of the implicated code, I will investigate. However, I cannot assign the bug to myself, as I'm not on the list of possible assignees.

This page was generated in about 0.008s by Fossil 2.27 [a6d506ecc4] 2025-07-09 13:52:53