Tk Source Code

View Ticket
Login
Ticket UUID: 2d3a81c0ecd1ccbc8c5d3063dfa6630e70b88266
Title: Read segmentation fault on Menu when destroyed in command and console output
Type: Bug Version: core-8-6-branch
Submitter: oehhar Created on: 2024-07-08 16:47:43
Subsystem: 13. Win Menus Assigned To: nobody
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2024-07-22 12:49:19
Resolution: Fixed Closed By: oehhar
    Closed on: 2024-07-22 12:49:19
Description:

For me, the following script crashes with segmentation fault, when the menubutton is opened and the checkbutton is selected.

pack [ttk::menubutton .b -text "Button"]
menu .b.s -tearoff 0
.b.s add checkbutton -label Selected -command selected
.b configure -menu .b.s

proc selected {} {
    destroy .b
    puts .
}

The crash does not happen, if one of the commands of the selected routine are removed.

Environment:

  • current tcl/tk branch core-8-6-branch or 8.6.14 release
  • Compiled by VisualStudio 2015 in x86 (32bit) mode
  • platform: MS_Windows 10 64 bit

The last call in Tk is in generic\tkMenu.c proc DestroyMenuEntry line 1450:

	Tcl_UntraceVar2(menuPtr->interp, varName, NULL,
		TCL_GLOBAL_ONLY|TCL_TRACE_WRITES|TCL_TRACE_UNSETS,
		MenuVarProc, mePtr);

The variable contents are:

menuPtr->interp : looks like arbitrary memory.
varName: String "fStop"
MenuVarProc -> pointer to a function
mePtr: current Menu entry

The menuPtr does not look good. It is the menu entry structure TkMenuEntry "menuPtr" element.

I suppose, that the destroy of the widget has already destroyed the menu. The menu item is preserved and its destruction fails, as the menu is already freed.

---

With "Tcl_UntraceVar2", TCL is called. The segmentation fault happens in the function "TclLookupSimpleVar()" in file "generic\tclVar.c" line 866:

    if ((cxtNsPtr->varResProc != NULL || iPtr->resolverPtr != NULL)
	    && !(flags & TCL_AVOID_RESOLVERS)) {

The invalid read access is in "cxtNsPtr->varResProc", which comes from "cxtNsPtr = iPtr->globalNsPtr;". "iPtr" is probably the current interpreter pointer and looks like invalid data.

This invalid data makes all the chain and is already present in the upper tk call.

The call stack is as follows:

>	tcl86tg.dll!TclLookupSimpleVar(Tcl_Interp * interp, Tcl_Obj * varNamePtr, int flags, int create, const char * * errMsgPtr, int * indexPtr) Zeile 866	C
 	tcl86tg.dll!TclObjLookupVarEx(Tcl_Interp * interp, Tcl_Obj * part1Ptr, Tcl_Obj * part2Ptr, int flags, const char * msg, int createPart1, int createPart2, Var * * arrayPtrPtr) Zeile 710	C
 	tcl86tg.dll!TclObjLookupVar(Tcl_Interp * interp, Tcl_Obj * part1Ptr, const char * part2, int flags, const char * msg, int createPart1, int createPart2, Var * * arrayPtrPtr) Zeile 520	C
 	tcl86tg.dll!TclLookupVar(Tcl_Interp * interp, const char * part1, const char * part2, int flags, const char * msg, int createPart1, int createPart2, Var * * arrayPtrPtr) Zeile 437	C
 	tcl86tg.dll!Tcl_UntraceVar2(Tcl_Interp * interp, const char * part1, const char * part2, int flags, char *(*)(void *, Tcl_Interp *, const char *, const char *, int) proc, void * clientData) Zeile 2919	C
 	tk86tg.dll!DestroyMenuEntry(char * memPtr) Zeile 1450	C
 	tcl86tg.dll!Tcl_Release(void * clientData) Zeile 229	C
 	tk86tg.dll!TkInvokeMenu(Tcl_Interp * interp, TkMenu * menuPtr, int index) Zeile 1064	C
 	tk86tg.dll!TkWinHandleMenuEvent(HWND__ * * dummy1141, unsigned int * pMessage, unsigned int * pwParam, long * plParam, long * plResult) Zeile 1243	C
 	tk86tg.dll!TkWinMenuProc(HWND__ * hwnd, unsigned int message, unsigned int wParam, long lParam) Zeile 1013	C
 	user32.dll!__InternalCallWinProc@20()	Unbekannt
 	user32.dll!UserCallWinProcCheckWow()	Unbekannt
 	user32.dll!_DispatchMessageWorker@8()	Unbekannt
 	user32.dll!_DispatchMessageW@4()	Unbekannt
 	tcl86tg.dll!Tcl_WaitForEvent(const Tcl_Time * timePtr) Zeile 518	C
 	tcl86tg.dll!Tcl_DoOneEvent(int flags) Zeile 946	C
 	tk86tg.dll!Tk_MainLoop() Zeile 2109	C
 	tk86tg.dll!Tk_MainExW(int argc, wchar_t * * argv, int(*)(Tcl_Interp *) appInitProc, Tcl_Interp * interp) Zeile 385	C
 	wish86tg.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpszCmdLine, int nCmdShow) Zeile 176	C
 	wish86tg.exe!invoke_main() Zeile 118	C++
 	wish86tg.exe!__scrt_common_main_seh() Zeile 253	C++
 	wish86tg.exe!__scrt_common_main() Zeile 296	C++
 	wish86tg.exe!wWinMainCRTStartup() Zeile 17	C++
 	kernel32.dll!@BaseThreadInitThunk@12()	Unbekannt
 	ntdll.dll!__RtlUserThreadStart()	Unbekannt
 	ntdll.dll!__RtlUserThreadStart@8()	Unbekannt

Please come back to me in case of any questions.

Thank you for any help, Harald

User Comments: oehhar added on 2024-07-22 12:49:19:

I can also confirm, that

  • the test in [f421344373] crashes wish 9.0b2
  • the merged branch of main does not crash

So, it was also ok, to merge this to the main branch.

Thank you all ! Harald


oehhar added on 2024-07-22 12:33:44:

Thanks, Francois, for the test and the opinion about the test case.

I merged to all branches by : 8.6:[fd7b2987], 8.7 [6c5b6b9a] and 9.0 [68d89887].

Cudos to Francois and Christian for the help !

Bug closed.

Harald


fvogel added on 2024-07-21 17:02:24:
About adding a test case: It's not easy to test this. We would need to exercise the code in TkWinHandleMenuEvent, which as its name says handles events from the Windows OS. Usually these kind of helper functions specific to the test suite are in tkTest.c but I don't think we already have code doing this.

Alternatively, following the example in, say, winMenu-11.8, we could write a testcase constrained by {win userInteraction} but I doubt it is worth the effort (does anyone run these userInteraction-constrained tests? how often?).

I suggest we just merge as is, without testcase.

fvogel added on 2024-07-21 16:50:19:

Confirmed [f421344373] is a duplicate of the present ticket.


oehhar added on 2024-07-19 13:15:56:

Ticket [f4213443] reports probably the same issue. The reproduction script was reproduced by 2 people. It might help to find a test case.


oehhar added on 2024-07-18 08:22:47:

Francois, CHristian,

thank you for the review and advice, I really appreciate. I feel so incapable here. And fossil strikes back either, sorry, for all this. But I had the reproductible crash and I did not want to let it unhandled.

Please find the following commits:

  • [d1624c46] original patch in tkMenu.c, but based to core-8-6-branch
  • [c08e58fc] proposal by Christian. Tk_Preserve in tkWinMenu.c, same way as in tMacOSXMenu.c. Fixes the issue and tk test suite is clean.
  • [7623fa7d] moves the comments to the header of TkInvokeMenu.

I think, that is in a good state now.

Any comments welcome.

Thanks, Harald


chw added on 2024-07-18 04:29:41:
Harald,

there are throughout the Tk source tree 3 call sites for TkInvokeMenu():

a) tkMenu.c for all platforms
b) tkWinMenu.c for Win32 specifically
c) tkMacOSXMenu.c for MacOS specifically

In the call sites a) and c) the Tcl_Preserve(menuPtr) and its Tcl_Release()
counterpart are already included outside of TkInvokeMenu().
But in b) not at all. Thus I propose to remove the preserve/release from
your patch in TkInvokeMenu() and add it instead in tkWinMenu.c to make
that call site look like a) and c).

BR,
Christian

fvogel added on 2024-07-17 22:15:06:

Sounds sensible, the explanation makes sense to me.

Note about your commit [bbade43e]: I guess it should rather be off core-8-6-branch, not off some old unrelated bugfix branch, right?


oehhar added on 2024-07-17 18:06:51:

I have made a debugging session again.

The menu is destroyed but not the entry, as it is preserved.

The stack trace of the menu destroy is as follows:

>	tk86tg.dll!DestroyMenuInstance(TkMenu * menuPtr) Zeile 1173	C
 	tk86tg.dll!TkDestroyMenu(TkMenu * menuPtr) Zeile 1271	C
 	tk86tg.dll!TkMenuEventProc(void * clientData, _XEvent * eventPtr) Zeile 776	C
 	tk86tg.dll!Tk_HandleEvent(_XEvent * eventPtr) Zeile 1270	C
 	tk86tg.dll!Tk_DestroyWindow(Tk_Window_ * tkwin) Zeile 1483	C
 	tk86tg.dll!Tk_DestroyWindow(Tk_Window_ * tkwin) Zeile 1424	C
 	tk86tg.dll!Tk_DestroyObjCmd(void * clientData, Tcl_Interp * interp, int objc, Tcl_Obj * const * objv) Zeile 500	C
 	tcl86tg.dll!Dispatch(void * * data, Tcl_Interp * interp, int result) Zeile 4505	C
 	tcl86tg.dll!TclNRRunCallbacks(Tcl_Interp * interp, int result, NRE_callback * rootPtr) Zeile 4541	C
 	tcl86tg.dll!TclEvalObjEx(Tcl_Interp * interp, Tcl_Obj * objPtr, int flags, const CmdFrame * invoker, int word) Zeile 6104	C
 	tcl86tg.dll!Tcl_EvalObjEx(Tcl_Interp * interp, Tcl_Obj * objPtr, int flags) Zeile 6085	C
 	tk86tg.dll!TkInvokeMenu(Tcl_Interp * interp, TkMenu * menuPtr, int index) Zeile 1061	C
 	tk86tg.dll!TkWinHandleMenuEvent(HWND__ * * dummy1141, unsigned int * pMessage, unsigned int * pwParam, long * plParam, long * plResult) Zeile 1243	C
 	tk86tg.dll!TkWinMenuProc(HWND__ * hwnd, unsigned int message, unsigned int wParam, long lParam) Zeile 1013	C
 

The debug log is as follows:

TkDestroyMenu(
    TkMenu *menuPtr)

> DestroyMenuInstance (tkMenu.c)
-> with correct menu pointer
> TkpDestroyMenu(menuPtr);  (tkWinMenu.c)
< back
1116: menuPtr->menuRefPtr != NULL -> continue
menuPtr->menuRefPtr->menuPtr = NULL; -> was my own menu

> TkFreeMenuReferences(menuPtr->menuRefPtr)
  Deletes hash entry
< back
... no cascade, no master
1173:
    for (i = menuPtr->numEntries; --i >= 0; ) {
	/*
	 * 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.
	 */

	Tcl_EventuallyFree(menuPtr->entries[i], DestroyMenuEntry);
	menuPtr->numEntries = i;
    }

> Tcl_EventuallyFree(
  There is a reference to the block, so
	refPtr->mustFree = 1;
	refPtr->freeProc = freeProc; (DestroyMenuEntry)
  DestroyMenu is not called...
<
    TkMenuFreeDrawOptions(menuPtr);
    Tk_FreeConfigOptions((char *) menuPtr,
	    tsdPtr->menuOptionTable, menuPtr->tkwin);

	Tk_Window tkwin = menuPtr->tkwin;

	menuPtr->tkwin = NULL;
	Tk_DestroyWindow(tkwin);
> Tk_DestroyWindow(tkwin); (tkWindow.c)
    if (winPtr->flags & TK_ALREADY_DEAD) {
	/*
	 * A destroy event binding caused the window to be destroyed again.
	 * Ignore the request.
	 */

	return;
    }

< Return from DestroyMenuInstance

Tcl_Release(menuPtr);
-> does not free menu pointer.

< Return from TkDestroyMenu

in TkMenuEventProc(

	    Tcl_DeleteCommandFromToken(menuPtr->interp, menuPtr->widgetCmd);
	    menuPtr->widgetCmd = NULL;

796: frees menuPtr in Tcl_EventuallyFree(menuPtr, TCL_DYNAMIC);

---

So, the menu entry is kept by the invoke process, but not the menu itself.

So, lets debug the invocation procedure:

>	tk86tg.dll!TkInvokeMenu(Tcl_Interp * interp, TkMenu * menuPtr, int index) Zeile 1008	C
 	tk86tg.dll!TkWinHandleMenuEvent(HWND__ * * dummy1141, unsigned int * pMessage, unsigned int * pwParam, long * plParam, long * plResult) Zeile 1243	C
 	tk86tg.dll!TkWinMenuProc(HWND__ * hwnd, unsigned int message, unsigned int wParam, long lParam) Zeile 1013	C
 	user32.dll!__InternalCallWinProc@20()	Unbekannt
 	user32.dll!UserCallWinProcCheckWow()	Unbekannt
 	user32.dll!_DispatchMessageWorker@8()	Unbekannt
 	user32.dll!_DispatchMessageW@4()	Unbekannt
 	tcl86tg.dll!Tcl_WaitForEvent(const Tcl_Time * timePtr) Zeile 518	C
 	tcl86tg.dll!Tcl_DoOneEvent(int flags) Zeile 946	C
 	tk86tg.dll!Tk_MainLoop() Zeile 2109	C
 	tk86tg.dll!Tk_MainExW(int argc, wchar_t * * argv, int(*)(Tcl_Interp *) appInitProc, Tcl_Interp * interp) Zeile 385	C
 	wish86tg.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpszCmdLine, int nCmdShow) Zeile 176	C
 	wish86tg.exe!invoke_main() Zeile 118	C++
 	wish86tg.exe!__scrt_common_main_seh() Zeile 253	C++
 	wish86tg.exe!__scrt_common_main() Zeile 296	C++
 	wish86tg.exe!wWinMainCRTStartup() Zeile 17	C++
 	kernel32.dll!@BaseThreadInitThunk@12()	Unbekannt
 	ntdll.dll!__RtlUserThreadStart()	Unbekannt
 	ntdll.dll!__RtlUserThreadStart@8()	Unbekannt

TkInvokeMenu (tkMenu.c 991)

    mePtr = menuPtr->entries[index];
    Tcl_Preserve(mePtr);
    -> this preserves the menu entry, but not the menu itself
    
    Call Command -> this will delete the menu, but not the entry, as it is preserved.

    Tcl_Release(mePtr);
    -> This will delete the menu entry with the menu gone
    -> as the menu is required for delete, we have the segmentation fault

So, the idea is also the preserve the menu. This is done in branch [ticket-2d3a81c0-menubutton-destroy-segfault] starting with commit [bbade43e].

For me, the code crashes always withoput this patch and works with this patch. Test suite is clean for me.

Any comments appreciated !

Thank you all, Harald


oehhar added on 2024-07-10 06:38:58:

Thank you, Francois, I appreciate ! As I have a workaround, it has lost priority. Thanks again, Harald


fvogel added on 2024-07-09 20:57:36:
Sorry I'm still not reproducing. Giving up.

oehhar added on 2024-07-09 09:24:29:

As a work around in the big application, I have added "after idle" after the -command argument to delay the widget destruction. This workaround helps.

Perhaps my wording below is quite unclear. The issue is still present and the recipe given in the post before triggers the issue for me.

Thank you, Harald


oehhar added on 2024-07-09 08:34:03:

I also tried with Tk9.0b2. It did not crash.


oehhar added on 2024-07-09 08:31:56:

Thanks, Francois, for the test, I appreciate. I also tried to modify the test script and replaced the puts by "update" or other options. It did not crash. Nevertheless, I see this crash in a large application.

Here is my exact recipe:

  • save the attached file "test.tcl" in a folder "c:\test"
  • start cmd.exe and go to this folder
  • invoke wish 8.6.14 with the file as argument: "C:\myprograms\tcl8.6\bin\wish86t.exe test.tcl"
  • The window opens. Press on the button to open the menu
  • Press on the only menu item "fStop"
  • The wish window dissappears - that is the crash. An empty window should remain, if it does not crash. You may test this by removing the "puts .".

To get the stack trace etc, I have build with symbols and program data base options and attached wish to VS2015. Then, you get the crash message "exception faul on read".

Please come back to me in case I may do any tests.

Thanks again, Harald


fvogel added on 2024-07-08 20:16:56:
I can't reproduce (Win11, Tcl and Tk from core-8-6-branch).

Attachments: