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:
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
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:
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:
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:
- test.tcl [download] added by oehhar on 2024-07-09 08:20:59. [details]