summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--ChangeLog8
-rw-r--r--generic/tkMenu.c198
-rw-r--r--generic/tkMenu.h22
-rw-r--r--generic/tkMenuDraw.c15
-rw-r--r--tests/menu.test4
5 files changed, 174 insertions, 73 deletions
diff --git a/ChangeLog b/ChangeLog
index 85e50fc..57e99f9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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}