Moved 'deletionEpoch' field from TkDisplay to TkMainInfo.

Reworked windowObj type.
Fixes Tk Bug #671330 "segfault when e.g. deiconifying destroyed window"

5 files changed, 79 insertions, 58 deletions

diff --git a/ChangeLog b/ChangeLog
index 1a04621..db862bd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2003-01-28  Joe English  <jenglish@users.sourceforge.net>
+	* generic/tkInt.h (TkDisplay,TkMainInfo):
+	* generic/tkObj.c (windowObjType):
+	* generic/tkWindow.c (Tk_DestroyWindow):
+	* tests/wm.test (wm-deletion-epoch-1.1):
+	Moved 'deletionEpoch' field from TkDisplay to TkMainInfo.
+	Reworked windowObj type.  Fixes Tk Bug #671330 "segfault when e.g.
+	deiconifying destroyed window"
+
 2003-01-23  D. Richard Hipp  <drh@hwaci.com>
 
 	* library/entry.tcl: Fix the KeyPress binding on the entry widget
diff --git a/generic/tkInt.h b/generic/tkInt.h
index f94c3ec..cd40d80 100644
--- a/generic/tkInt.h
+++ b/generic/tkInt.h
@@ -11,7 +11,7 @@
  * See the file "license.terms" for information on usage and redistribution
  * of this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: $Id: tkInt.h,v 1.54 2002/08/31 06:12:20 das Exp $ 
+ * RCS: $Id: tkInt.h,v 1.55 2003/01/28 20:39:15 jenglish Exp $ 
  */
 
 #ifndef _TKINT
@@ -500,7 +500,6 @@ typedef struct TkDisplay {
     /*
      * The following field(s) were all added for Tk8.4
      */
-    long deletionEpoch;		/* Incremented by window deletions */
     unsigned int flags;		/* Various flag values:  these are all
 				 * defined in below. */
     TkCaret caret;		/* information about the caret for this
@@ -601,6 +600,7 @@ typedef struct TkMainInfo {
     Tcl_HashTable nameTable;	/* Hash table mapping path names to TkWindow
 				 * structs for all windows related to this
 				 * main window.  Managed by tkWindow.c. */
+    long deletionEpoch;		/* Incremented by window deletions */
     Tk_BindingTable bindingTable;
 				/* Used in conjunction with "bind" command
 				 * to bind events to Tcl commands. */
diff --git a/generic/tkObj.c b/generic/tkObj.c
index 26d5f79..fff25d9 100644
--- a/generic/tkObj.c
+++ b/generic/tkObj.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: tkObj.c,v 1.7 2001/08/21 14:43:08 dkf Exp $
+ * RCS: @(#) $Id: tkObj.c,v 1.8 2003/01/28 20:39:16 jenglish Exp $
  */
 
 #include "tkInt.h"
@@ -56,12 +56,14 @@ typedef struct MMRep {
 
 /*
  * The following structure is the internal representation for window objects.
+ * A WindowRep caches name-to-window lookups.  The cache is invalid
+ * if tkwin is NULL or if mainPtr->deletionEpoch does not match epoch.
  */
- 
 typedef struct WindowRep {
-    Tk_Window tkwin;
-    Tk_Window mainwin;
-    long epoch;
+    Tk_Window tkwin;		/* Cached window; NULL if not found */
+    TkMainInfo *mainPtr;	/* MainWindow associated with tkwin */
+    long epoch;			/* Value of mainPtr->deletionEpoch at last
+				 * successful lookup.  */
 } WindowRep;
 
 /*
@@ -221,6 +223,7 @@ FreePixelInternalRep(objPtr)
 	ckfree((char *) pixelPtr);
     }
     SET_SIMPLEPIXEL(objPtr, 0);
+    objPtr->typePtr = NULL;
 }
 
 /*
@@ -447,6 +450,7 @@ FreeMMInternalRep(objPtr)
 {
     ckfree((char *) objPtr->internalRep.otherValuePtr);
     objPtr->internalRep.otherValuePtr = NULL;
+    objPtr->typePtr = NULL;
 }
 
 /*
@@ -679,41 +683,38 @@ int
 TkGetWindowFromObj(interp, tkwin, objPtr, windowPtr)
     Tcl_Interp *interp; 	/* Used for error reporting if not NULL. */
     Tk_Window tkwin;		/* A token to get the main window from. */
-    Tcl_Obj *objPtr;		/* The object from which to get boolean. */
+    Tcl_Obj *objPtr;		/* The object from which to get window. */
     Tk_Window *windowPtr;	/* Place to store resulting window. */
 {
+    TkMainInfo *mainPtr = ((TkWindow *)tkwin)->mainPtr;
     register WindowRep *winPtr;
-    TkDisplay *dispPtr = ((TkWindow *)tkwin)->dispPtr;
-    Tk_Window foundWindow;
+    int result;
 
-    if (objPtr->typePtr != &windowObjType) {
-	register int result = SetWindowFromAny(interp, objPtr);
-	if (result != TCL_OK) {
-	    return result;
-	}
+    result = Tcl_ConvertToType(interp, objPtr, &windowObjType);
+    if (result != TCL_OK) {
+	return result;
     }
 
     winPtr = (WindowRep *) objPtr->internalRep.otherValuePtr;
-    if (winPtr == NULL) {
-	winPtr = (WindowRep *) ckalloc(sizeof(WindowRep));
-	objPtr->internalRep.otherValuePtr = (VOID *) winPtr;
-	goto parseWindowString;
-
-    } else if (tkwin != winPtr->mainwin ||
-	       dispPtr->deletionEpoch != winPtr->epoch) {
-    parseWindowString:
-	foundWindow = Tk_NameToWindow(interp,
+    if (    winPtr->tkwin == NULL
+	 || winPtr->mainPtr == NULL
+	 || winPtr->mainPtr != mainPtr 
+	 || winPtr->epoch != mainPtr->deletionEpoch) 
+    {
+	/* Cache is invalid.
+	 */
+	winPtr->tkwin = Tk_NameToWindow(interp,
 		Tcl_GetStringFromObj(objPtr, NULL), tkwin);
-	if (foundWindow == NULL) {
-	    return TCL_ERROR;
-	}
-
-	winPtr->tkwin = foundWindow;
-	winPtr->mainwin = tkwin;
-	winPtr->epoch = dispPtr->deletionEpoch;
+	winPtr->mainPtr = mainPtr;
+	winPtr->epoch = mainPtr ? mainPtr->deletionEpoch : 0;
     }
 
     *windowPtr = winPtr->tkwin;
+
+    if (winPtr->tkwin == NULL) {
+	/* ASSERT: Tk_NameToWindow has left error message in interp */
+	return TCL_ERROR;
+    }
     return TCL_OK;
 }
 
@@ -721,18 +722,17 @@ TkGetWindowFromObj(interp, tkwin, objPtr, windowPtr)
  *----------------------------------------------------------------------
  *
  * SetWindowFromAny --
- *
- *	Attempt to generate a Tk_Window internal form for the Tcl object
- *	"objPtr".
+ *	Generate a windowObj internal form for the Tcl object "objPtr".
  *
  * Results:
- *	The return value is a standard Tcl result. If an error occurs during
- *	conversion, an error message is left in the interpreter's result
- *	unless "interp" is NULL.
+ *	Always returns TCL_OK. 
  *
  * Side effects:
- *	If no error occurs, a standard window value is stored as "objPtr"s
- *	internal representation and the type of "objPtr" is set to Tk_Window.
+ *	Sets objPtr's internal representation to an uninitialized
+ *	windowObj. Frees the old internal representation, if any.
+ *
+ * See also:
+ * 	TkGetWindowFromObj, which initializes the WindowRep cache.
  *
  *----------------------------------------------------------------------
  */
@@ -743,6 +743,7 @@ SetWindowFromAny(interp, objPtr)
     register Tcl_Obj *objPtr;	/* The object to convert. */
 {
     Tcl_ObjType *typePtr;
+    WindowRep *winPtr;
 
     /*
      * Free the old internalRep before setting the new one. 
@@ -753,8 +754,14 @@ SetWindowFromAny(interp, objPtr)
     if ((typePtr != NULL) && (typePtr->freeIntRepProc != NULL)) {
 	(*typePtr->freeIntRepProc)(objPtr);
     }
+
+    winPtr = (WindowRep *) ckalloc(sizeof(WindowRep));
+    winPtr->tkwin = NULL;
+    winPtr->mainPtr = NULL;
+    winPtr->epoch = 0;
+
+    objPtr->internalRep.otherValuePtr = (VOID*)winPtr;
     objPtr->typePtr = &windowObjType;
-    objPtr->internalRep.otherValuePtr = NULL;
 
     return TCL_OK;
 }
@@ -784,17 +791,13 @@ DupWindowInternalRep(srcPtr, copyPtr)
 {
     register WindowRep *oldPtr, *newPtr;
 
-    copyPtr->typePtr = srcPtr->typePtr;
     oldPtr = srcPtr->internalRep.otherValuePtr;
-    if (oldPtr == NULL) {
-	copyPtr->internalRep.otherValuePtr = NULL;
-    } else {
-	newPtr = (WindowRep *) ckalloc(sizeof(WindowRep));
-	newPtr->tkwin = oldPtr->tkwin;
-	newPtr->mainwin = oldPtr->mainwin;
-	newPtr->epoch = oldPtr->epoch;
-	copyPtr->internalRep.otherValuePtr = (VOID *)newPtr;
-    }
+    newPtr = (WindowRep *) ckalloc(sizeof(WindowRep));
+    newPtr->tkwin = oldPtr->tkwin;
+    newPtr->mainPtr = oldPtr->mainPtr;
+    newPtr->epoch = oldPtr->epoch;
+    copyPtr->internalRep.otherValuePtr = (VOID *)newPtr;
+    copyPtr->typePtr = srcPtr->typePtr;
 }
 
 /*
@@ -819,10 +822,9 @@ static void
 FreeWindowInternalRep(objPtr)
     Tcl_Obj *objPtr;		/* Window object with internal rep to free. */
 {
-    if (objPtr->internalRep.otherValuePtr != NULL) {
-	ckfree((char *) objPtr->internalRep.otherValuePtr);
-	objPtr->internalRep.otherValuePtr = NULL;
-    }
+    ckfree((char *) objPtr->internalRep.otherValuePtr);
+    objPtr->internalRep.otherValuePtr = NULL;
+    objPtr->typePtr = NULL;
 }
 
 /*
diff --git a/generic/tkWindow.c b/generic/tkWindow.c
index dc75118..c1e6499 100644
--- a/generic/tkWindow.c
+++ b/generic/tkWindow.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: tkWindow.c,v 1.55 2002/11/14 17:30:20 mdejong Exp $
+ * RCS: @(#) $Id: tkWindow.c,v 1.56 2003/01/28 20:39:17 jenglish Exp $
  */
 
 #include "tkPort.h"
@@ -888,6 +888,7 @@ TkCreateMainWindow(interp, screenName, baseName)
     mainPtr->refCount = 1;
     mainPtr->interp = interp;
     Tcl_InitHashTable(&mainPtr->nameTable, TCL_STRING_KEYS);
+    mainPtr->deletionEpoch = 0l;
     TkEventInit();
     TkBindInit(mainPtr);
     TkFontPkgInit(mainPtr);
@@ -1493,9 +1494,10 @@ Tk_DestroyWindow(tkwin)
             winPtr->pathName = NULL;
 
 	    /*
-	     * Invalidate all objects referring to windows on this display.
+	     * Invalidate all objects referring to windows
+	     * with the same main window
 	     */
-	    dispPtr->deletionEpoch++;
+	    winPtr->mainPtr->deletionEpoch++;
 	}
 	winPtr->mainPtr->refCount--;
 	if (winPtr->mainPtr->refCount == 0) {
diff --git a/tests/wm.test b/tests/wm.test
index 145ee3f..e9d2de5 100644
--- a/tests/wm.test
+++ b/tests/wm.test
@@ -7,7 +7,7 @@
 # Copyright (c) 1998-1999 by Scriptics Corporation.
 # All rights reserved.
 #
-# RCS: @(#) $Id: wm.test,v 1.20 2002/12/01 23:37:53 mdejong Exp $
+# RCS: @(#) $Id: wm.test,v 1.21 2003/01/28 20:39:19 jenglish Exp $
 
 # This file tests window manager interactions that work across
 # platforms. Window manager tests that only work on a specific
@@ -1635,6 +1635,14 @@ test wm-withdraw-3.1 {} {
     lappend result [wm state .t] [winfo ismapped .t]
 } {withdrawn 0 normal 1}
 
+test wm-deletion-epoch-1.1 {Deletion epoch on multiple displays} {altDisplay} {
+    # See Tk Bug #671330 "segfault when e.g. deiconifying destroyed window"
+    deleteWindows
+    set w [toplevel .t -screen $env(TK_ALT_DISPLAY)]
+    wm deiconify $w         ;# this caches the WindowRep
+    destroy .t
+    list [catch {wm deiconify $w} msg] $msg
+} {1 {bad window path name ".t"}}
 
 # FIXME: