diff options
| -rw-r--r-- | ChangeLog | 8 | ||||
| -rw-r--r-- | generic/tkMenu.c | 198 | ||||
| -rw-r--r-- | generic/tkMenu.h | 22 | ||||
| -rw-r--r-- | generic/tkMenuDraw.c | 15 | ||||
| -rw-r--r-- | tests/menu.test | 4 |
5 files changed, 174 insertions, 73 deletions
@@ -1,3 +1,11 @@ +2003-05-30 Vince Darley <vincentdarley@users.sourceforge.net> + + * generic/tkMenu.c + * generic/tkMenu.h + * generic/tkMenuDraw.c: + * tests/menu.test: fixed complex bug in menu clone cleanup + [Bug#465324] and removed 'knownBug' from a test. + 2003-05-29 Joe English <jenglish@users.sourceforge.net> * doc/text.n: Fixed markup errors. diff --git a/generic/tkMenu.c b/generic/tkMenu.c index 152e57f..713a288 100644 --- a/generic/tkMenu.c +++ b/generic/tkMenu.c @@ -12,7 +12,7 @@ * See the file "license.terms" for information on usage and redistribution * of this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkMenu.c,v 1.20 2003/02/26 02:32:56 hobbs Exp $ + * RCS: @(#) $Id: tkMenu.c,v 1.21 2003/05/30 11:03:00 vincentdarley Exp $ */ /* @@ -496,7 +496,9 @@ MenuCmd(clientData, interp, objc, objv) } /* - * Initialize the data structure for the menu. + * Initialize the data structure for the menu. Note that the + * menuPtr is eventually freed in 'TkMenuEventProc' in tkMenuDraw.c, + * when Tcl_EventuallyFree is called. */ menuPtr = (TkMenu *) ckalloc(sizeof(TkMenu)); @@ -1186,7 +1188,9 @@ DestroyMenuInstance(menuPtr) TkpDestroyMenu(menuPtr); cascadePtr = menuPtr->menuRefPtr->parentEntryPtr; menuPtr->menuRefPtr->menuPtr = NULL; - TkFreeMenuReferences(menuPtr->menuRefPtr); + if (TkFreeMenuReferences(menuPtr->menuRefPtr)) { + menuPtr->menuRefPtr = NULL; + } for (; cascadePtr != NULL; cascadePtr = nextCascadePtr) { nextCascadePtr = cascadePtr->nextCascadePtr; @@ -1252,6 +1256,11 @@ DestroyMenuInstance(menuPtr) TkMenuFreeDrawOptions(menuPtr); Tk_FreeConfigOptions((char *) menuPtr, menuPtr->optionTablesPtr->menuOptionTable, menuPtr->tkwin); + if (menuPtr->tkwin != NULL) { + Tk_Window tkwin = menuPtr->tkwin; + menuPtr->tkwin = NULL; + Tk_DestroyWindow(tkwin); + } } /* @@ -1285,6 +1294,8 @@ TkDestroyMenu(menuPtr) return; } + Tcl_Preserve(menuPtr); + /* * Now destroy all non-tearoff instances of this menu if this is a * parent menu. Is this loop safe enough? Are there going to be @@ -1292,30 +1303,38 @@ TkDestroyMenu(menuPtr) * we have to do a slightly more complex scheme. */ + menuPtr->menuFlags |= MENU_DELETION_PENDING; + if (menuPtr->menuRefPtr != NULL) { + /* + * If any toplevel widgets have this menu as their menubar, + * the geometry of the window may have to be recalculated. + */ + topLevelListPtr = menuPtr->menuRefPtr->topLevelListPtr; + while (topLevelListPtr != NULL) { + nextTopLevelPtr = topLevelListPtr->nextPtr; + TkpSetWindowMenuBar(topLevelListPtr->tkwin, NULL); + topLevelListPtr = nextTopLevelPtr; + } + } if (menuPtr->masterMenuPtr == menuPtr) { - menuPtr->menuFlags |= MENU_DELETION_PENDING; while (menuPtr->nextInstancePtr != NULL) { menuInstancePtr = menuPtr->nextInstancePtr; menuPtr->nextInstancePtr = menuInstancePtr->nextInstancePtr; if (menuInstancePtr->tkwin != NULL) { - Tk_DestroyWindow(menuInstancePtr->tkwin); + Tk_Window tkwin = menuInstancePtr->tkwin; + /* + * Note: it may be desirable to NULL out the tkwin + * field of menuInstancePtr here: + * menuInstancePtr->tkwin = NULL; + */ + Tk_DestroyWindow(tkwin); } } - menuPtr->menuFlags &= ~MENU_DELETION_PENDING; } - /* - * If any toplevel widgets have this menu as their menubar, - * the geometry of the window may have to be recalculated. - */ - - topLevelListPtr = menuPtr->menuRefPtr->topLevelListPtr; - while (topLevelListPtr != NULL) { - nextTopLevelPtr = topLevelListPtr->nextPtr; - TkpSetWindowMenuBar(topLevelListPtr->tkwin, NULL); - topLevelListPtr = nextTopLevelPtr; - } DestroyMenuInstance(menuPtr); + + Tcl_Release(menuPtr); } /* @@ -1326,6 +1345,10 @@ TkDestroyMenu(menuPtr) * This entry is removed from the list of entries that point to the * cascade menu. This is done in preparation for changing the menu * that this entry points to. + * + * At the end of this function, the menu entry no longer contains + * a reference to a 'TkMenuReferences' structure, and therefore + * no such structure contains a reference to this menu entry either. * * Results: * None @@ -1352,6 +1375,8 @@ UnhookCascadeEntry(mePtr) cascadeEntryPtr = menuRefPtr->parentEntryPtr; if (cascadeEntryPtr == NULL) { + TkFreeMenuReferences(menuRefPtr); + mePtr->childMenuRefPtr = NULL; return; } @@ -1370,6 +1395,10 @@ UnhookCascadeEntry(mePtr) */ menuRefPtr->parentEntryPtr = NULL; + /* + * The original field is set to zero below, after it is + * freed. + */ TkFreeMenuReferences(menuRefPtr); } else { menuRefPtr->parentEntryPtr = cascadeEntryPtr->nextCascadePtr; @@ -1388,6 +1417,7 @@ UnhookCascadeEntry(mePtr) break; } } + mePtr->nextCascadePtr = NULL; } mePtr->childMenuRefPtr = NULL; } @@ -1435,7 +1465,40 @@ DestroyMenuEntry(memPtr) */ if (mePtr->type == CASCADE_ENTRY) { - UnhookCascadeEntry(mePtr); + if (menuPtr->masterMenuPtr != menuPtr) { + TkMenu *destroyThis = NULL; + /* + * The menu as a whole is a clone. We must delete the clone + * of the cascaded menu for the particular entry we are + * destroying. + */ + TkMenuReferences *menuRefPtr = mePtr->childMenuRefPtr; + if (menuRefPtr != NULL) { + destroyThis = menuRefPtr->menuPtr; + /* + * But only if it is a clone. What can happen is that + * we are in the middle of deleting a menu and this + * menu pointer has already been reset to point to the + * original menu. In that case we have nothing special + * to do. + */ + if ((destroyThis != NULL) + && (destroyThis->masterMenuPtr == destroyThis)) { + destroyThis = NULL; + } + } + UnhookCascadeEntry(mePtr); + if (menuRefPtr != NULL) { + if (menuRefPtr->menuPtr == destroyThis) { + menuRefPtr->menuPtr = NULL; + } + if (destroyThis != NULL) { + TkDestroyMenu(destroyThis); + } + } + } else { + UnhookCascadeEntry(mePtr); + } } if (mePtr->image != NULL) { Tk_FreeImage(mePtr->image); @@ -2198,6 +2261,11 @@ MenuCmdDeletedProc(clientData) */ if (tkwin != NULL) { + /* + * Note: it may be desirable to NULL out the tkwin + * field of menuPtr here: + * menuPtr->tkwin = NULL; + */ Tk_DestroyWindow(tkwin); } } @@ -2935,6 +3003,13 @@ RecursivelyDeleteMenu(menuPtr) int i; TkMenuEntry *mePtr; + /* + * It is not 100% clear that this preserve/release pair is + * required, but we have added them for safety in this + * very complex code. + */ + Tcl_Preserve(menuPtr); + for (i = 0; i < menuPtr->numEntries; i++) { mePtr = menuPtr->entries[i]; if ((mePtr->type == CASCADE_ENTRY) @@ -2943,7 +3018,11 @@ RecursivelyDeleteMenu(menuPtr) RecursivelyDeleteMenu(mePtr->childMenuRefPtr->menuPtr); } } - Tk_DestroyWindow(menuPtr->tkwin); + if (menuPtr->tkwin != NULL) { + Tk_DestroyWindow(menuPtr->tkwin); + } + + Tcl_Release(menuPtr); } /* @@ -3095,18 +3174,14 @@ TkSetWindowMenuBar(interp, tkwin, oldMenuName, menuName) * that reference this menu. */ - for (topLevelListPtr = menuRefPtr->topLevelListPtr, - prevTopLevelPtr = NULL; - (topLevelListPtr != NULL) - && (topLevelListPtr->tkwin != tkwin); - prevTopLevelPtr = topLevelListPtr, - topLevelListPtr = topLevelListPtr->nextPtr) { - - /* - * Empty loop body. - */ - - } + topLevelListPtr = menuRefPtr->topLevelListPtr; + prevTopLevelPtr = NULL; + + while ((topLevelListPtr != NULL) + && (topLevelListPtr->tkwin != tkwin)) { + prevTopLevelPtr = topLevelListPtr; + topLevelListPtr = topLevelListPtr->nextPtr; + } /* * Now we have found the toplevel reference that matches the @@ -3377,7 +3452,8 @@ TkFindMenuReferencesObj(interp, objPtr) * is cleared. It cleans up the ref if it is now empty. * * Results: - * None. + * Returns 1 if the references structure was freed, and 0 + * otherwise. * * Side effects: * If this is the last field to be cleared, the menu ref is @@ -3386,7 +3462,7 @@ TkFindMenuReferencesObj(interp, objPtr) *---------------------------------------------------------------------- */ -void +int TkFreeMenuReferences(menuRefPtr) TkMenuReferences *menuRefPtr; /* The menu reference to * free */ @@ -3396,7 +3472,9 @@ TkFreeMenuReferences(menuRefPtr) && (menuRefPtr->topLevelListPtr == NULL)) { Tcl_DeleteHashEntry(menuRefPtr->hashEntryPtr); ckfree((char *) menuRefPtr); + return 1; } + return 0; } /* @@ -3453,28 +3531,28 @@ DeleteMenuCloneEntries(menuPtr, first, last) } } -/* - *---------------------------------------------------------------------- - * - * TkMenuCleanup -- - * - * Resets menusInitialized to allow Tk to be finalized and reused - * without the DLL being unloaded. - * - * Results: - * None. - * - * Side effects: - * None. - * - *---------------------------------------------------------------------- - */ - -static void -TkMenuCleanup(ClientData unused) -{ - menusInitialized = 0; -} +/* + *---------------------------------------------------------------------- + * + * TkMenuCleanup -- + * + * Resets menusInitialized to allow Tk to be finalized and reused + * without the DLL being unloaded. + * + * Results: + * None. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + +static void +TkMenuCleanup(ClientData unused) +{ + menusInitialized = 0; +} /* *---------------------------------------------------------------------- @@ -3498,17 +3576,17 @@ TkMenuInit() { ThreadSpecificData *tsdPtr = (ThreadSpecificData *) Tcl_GetThreadData(&dataKey, sizeof(ThreadSpecificData)); - + if (!menusInitialized) { Tcl_MutexLock(&menuMutex); if (!menusInitialized) { TkpMenuInit(); menusInitialized = 1; } - /* - * Make sure we cleanup on finalize. - */ - Tcl_CreateExitHandler((Tcl_ExitProc *) TkMenuCleanup, NULL); + /* + * Make sure we cleanup on finalize. + */ + Tcl_CreateExitHandler((Tcl_ExitProc *) TkMenuCleanup, NULL); Tcl_MutexUnlock(&menuMutex); } if (!tsdPtr->menusInitialized) { diff --git a/generic/tkMenu.h b/generic/tkMenu.h index 9553bc0..42a6719 100644 --- a/generic/tkMenu.h +++ b/generic/tkMenu.h @@ -8,7 +8,7 @@ * See the file "license.terms" for information on usage and redistribution * of this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkMenu.h,v 1.6 2001/10/12 13:30:31 tmh Exp $ + * RCS: @(#) $Id: tkMenu.h,v 1.7 2003/05/30 11:03:00 vincentdarley Exp $ */ #ifndef _TKMENU @@ -451,17 +451,21 @@ typedef struct TkMenuOptionTables { * RESIZE_PENDING: Non-zero means a call to ComputeMenuGeometry * has already been scheduled. * MENU_DELETION_PENDING Non-zero means that we are currently destroying - * this menu. This is useful when we are in the - * middle of cleaning this master menu's chain of - * menus up when TkDestroyMenu was called again on - * this menu (via a destroy binding or somesuch). + * this menu's internal structures. This is useful + * when we are in the middle of cleaning + * this master menu's chain of menus up when + * TkDestroyMenu was called again on this + * menu (via a destroy binding or somesuch). + * MENU_WIN_DESTRUCTION_PENDING Non-zero means we are in the middle of + * destroying this menu's Tk_Window. * MENU_PLATFORM_FLAG1... Reserved for use by the platform-specific menu * code. */ -#define REDRAW_PENDING 1 -#define RESIZE_PENDING 2 -#define MENU_DELETION_PENDING 4 +#define REDRAW_PENDING 1 +#define RESIZE_PENDING 2 +#define MENU_DELETION_PENDING 4 +#define MENU_WIN_DESTRUCTION_PENDING 8 #define MENU_PLATFORM_FLAG1 (1 << 30) #define MENU_PLATFORM_FLAG2 (1 << 29) #define MENU_PLATFORM_FLAG3 (1 << 28) @@ -511,7 +515,7 @@ EXTERN TkMenuReferences * EXTERN TkMenuReferences * TkFindMenuReferencesObj _ANSI_ARGS_(( Tcl_Interp *interp, Tcl_Obj *namePtr)); -EXTERN void TkFreeMenuReferences _ANSI_ARGS_(( +EXTERN int TkFreeMenuReferences _ANSI_ARGS_(( TkMenuReferences *menuRefPtr)); EXTERN Tcl_HashTable * TkGetMenuHashTable _ANSI_ARGS_((Tcl_Interp *interp)); EXTERN int TkGetMenuIndex _ANSI_ARGS_((Tcl_Interp *interp, diff --git a/generic/tkMenuDraw.c b/generic/tkMenuDraw.c index 42cdf43..0274ac7 100644 --- a/generic/tkMenuDraw.c +++ b/generic/tkMenuDraw.c @@ -9,7 +9,7 @@ * See the file "license.terms" for information on usage and redistribution * of this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkMenuDraw.c,v 1.3 1999/04/16 01:51:19 stanton Exp $ + * RCS: @(#) $Id: tkMenuDraw.c,v 1.4 2003/05/30 11:03:00 vincentdarley Exp $ */ #include "tkMenu.h" @@ -769,15 +769,26 @@ TkMenuEventProc(clientData, eventPtr) } } else if (eventPtr->type == DestroyNotify) { if (menuPtr->tkwin != NULL) { - TkDestroyMenu(menuPtr); + if (!(menuPtr->menuFlags & MENU_DELETION_PENDING)) { + TkDestroyMenu(menuPtr); + } menuPtr->tkwin = NULL; + } + if (menuPtr->menuFlags & MENU_WIN_DESTRUCTION_PENDING) { + return; + } + menuPtr->menuFlags |= MENU_WIN_DESTRUCTION_PENDING; + if (menuPtr->widgetCmd != NULL) { Tcl_DeleteCommandFromToken(menuPtr->interp, menuPtr->widgetCmd); + menuPtr->widgetCmd = NULL; } if (menuPtr->menuFlags & REDRAW_PENDING) { Tcl_CancelIdleCall(DisplayMenu, (ClientData) menuPtr); + menuPtr->menuFlags &= ~REDRAW_PENDING; } if (menuPtr->menuFlags & RESIZE_PENDING) { Tcl_CancelIdleCall(ComputeMenuGeometry, (ClientData) menuPtr); + menuPtr->menuFlags &= ~RESIZE_PENDING; } Tcl_EventuallyFree((ClientData) menuPtr, TCL_DYNAMIC); } diff --git a/tests/menu.test b/tests/menu.test index 30d38dc..d5d93d0 100644 --- a/tests/menu.test +++ b/tests/menu.test @@ -5,7 +5,7 @@ # Copyright (c) 1998-1999 by Scriptics Corporation. # All rights reserved. # -# RCS: @(#) $Id: menu.test,v 1.14 2003/04/01 21:06:42 dgp Exp $ +# RCS: @(#) $Id: menu.test,v 1.15 2003/05/30 11:03:02 vincentdarley Exp $ package require tcltest 2.1 eval tcltest::configure $argv @@ -2420,7 +2420,7 @@ test menu-32.7 {DeleteMenuCloneEntries - one entry} { .m1 add command -label Hello list [catch {.m1 delete Hello} msg] $msg [destroy .m1] } {0 {} {}} -test menu-32.8 {Ensure all menu clone commands are deleted} {knownBug} { +test menu-32.8 {Ensure all menu clone commands are deleted} { # SF bug #465324 catch {destroy .menubar} catch {destroy .menubar.test} |