Fixed a memory leak when a placed widget was forgotten. [Bug 1028888]

3 files changed, 92 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index 7b8e13d..b476512 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2004-09-16  Peter Spjuth  <peter.spjuth@space.se>
+
+	* tests/place.test:
+	* generic/tkPlace.c: Fixed a memory leak when a placed widget was
+	forgotten. [Bug 1028888]
+
 2004-09-14  Mo DeJong  <mdejong@users.sourceforge.net>
 
 	* win/tkWinWm.c (WmIconwindowCmd): Replace bogus
diff --git a/generic/tkPlace.c b/generic/tkPlace.c
index dacdcea..bdd0a7d 100644
--- a/generic/tkPlace.c
+++ b/generic/tkPlace.c
@@ -10,7 +10,7 @@
  * See the file "license.terms" for information on usage and redistribution
  * of this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tkPlace.c,v 1.15 2004/01/13 02:06:00 davygrvy Exp $
+ * RCS: @(#) $Id: tkPlace.c,v 1.16 2004/09/16 18:01:20 pspjuth Exp $
  */
 
 #include "tkPort.h"
@@ -49,6 +49,8 @@ typedef struct Slave {
 				 * master was deleted or never assigned. */
     struct Slave *nextPtr;	/* Next in list of windows placed relative
 				 * to same master (NULL for end of list). */
+    Tk_OptionTable optionTable;	/* Table that defines configuration options
+				 * available for this command. */
     /*
      * Geometry information for window;  where there are both relative
      * and absolute values for the same attribute (e.g. x and relX) only
@@ -170,7 +172,9 @@ static int		ConfigureSlave _ANSI_ARGS_((Tcl_Interp *interp,
 			    int objc, Tcl_Obj *CONST objv[]));
 static int		PlaceInfoCommand _ANSI_ARGS_((Tcl_Interp *interp,
 			    Tk_Window tkwin));
-static Slave *		CreateSlave _ANSI_ARGS_((Tk_Window tkwin));
+static Slave *		CreateSlave _ANSI_ARGS_((Tk_Window tkwin,
+			    Tk_OptionTable table));
+static void             FreeSlave _ANSI_ARGS_((Slave *slavePtr));
 static Slave *		FindSlave _ANSI_ARGS_((Tk_Window tkwin));
 static Master *		CreateMaster _ANSI_ARGS_((Tk_Window tkwin));
 static Master *		FindMaster _ANSI_ARGS_((Tk_Window tkwin));
@@ -226,7 +230,7 @@ Tk_PlaceObjCmd(clientData, interp, objc, objv)
      * been created, the cached pointer will be returned.
      */
 
-    optionTable = Tk_CreateOptionTable(interp, optionSpecs);
+     optionTable = Tk_CreateOptionTable(interp, optionSpecs);
 
     /*
      * Handle special shortcut where window name is first argument.
@@ -325,7 +329,7 @@ Tk_PlaceObjCmd(clientData, interp, objc, objv)
 		    SlaveStructureProc,	(ClientData) slavePtr);
 	    Tk_ManageGeometry(tkwin, (Tk_GeomMgr *) NULL, (ClientData) NULL);
 	    Tk_UnmapWindow(tkwin);
-	    ckfree((char *) slavePtr);
+	    FreeSlave(slavePtr);
 	    break;
 	}
 	
@@ -379,8 +383,9 @@ Tk_PlaceObjCmd(clientData, interp, objc, objv)
  */
 
 static Slave *
-CreateSlave(tkwin)
+CreateSlave(tkwin, table)
     Tk_Window tkwin;		/* Token for desired slave. */
+    Tk_OptionTable table;
 {
     Tcl_HashEntry *hPtr;
     register Slave *slavePtr;
@@ -395,6 +400,7 @@ CreateSlave(tkwin)
 	slavePtr->inTkwin	= None;
 	slavePtr->anchor	= TK_ANCHOR_NW;
 	slavePtr->borderMode	= BM_INSIDE;
+	slavePtr->optionTable   = table;
 	Tcl_SetHashValue(hPtr, slavePtr);
 	Tk_CreateEventHandler(tkwin, StructureNotifyMask, SlaveStructureProc,
 		(ClientData) slavePtr);
@@ -403,6 +409,32 @@ CreateSlave(tkwin)
     }
     return slavePtr;
 }
+
+
+/*
+ *----------------------------------------------------------------------
+ *
+ * FreeSlave --
+ *
+ *	Frees the resources held by a Slave structure.
+ *
+ * Results:
+ *	None
+ *
+ * Side effects:
+ *	Memory are freed.
+ *
+ *----------------------------------------------------------------------
+ */
+
+static void
+FreeSlave(Slave *slavePtr)
+{
+    Tk_FreeConfigOptions((char *) slavePtr, slavePtr->optionTable,
+	    slavePtr->tkwin);
+    ckfree((char *) slavePtr);
+}
+
 
 /*
  *----------------------------------------------------------------------
@@ -599,9 +631,9 @@ ConfigureSlave(interp, tkwin, table, objc, objv)
 	return TCL_ERROR;
     }
 
-    slavePtr = CreateSlave(tkwin);
+    slavePtr = CreateSlave(tkwin, table);
 
-    if (Tk_SetOptions(interp, (char *)slavePtr, table, objc, objv,
+    if (Tk_SetOptions(interp, (char *) slavePtr, table, objc, objv,
 	    slavePtr->tkwin, &savedOptions, &mask) != TCL_OK) {
 	goto error;
     }
@@ -1100,7 +1132,7 @@ SlaveStructureProc(clientData, eventPtr)
 	}
 	Tcl_DeleteHashEntry(Tcl_FindHashEntry(&dispPtr->slaveTable,
 		(char *) slavePtr->tkwin));
-	ckfree((char *) slavePtr);
+	FreeSlave(slavePtr);
     }
 }
 
@@ -1182,5 +1214,5 @@ PlaceLostSlaveProc(clientData, tkwin)
             (char *) tkwin));
     Tk_DeleteEventHandler(tkwin, StructureNotifyMask, SlaveStructureProc,
 	    (ClientData) slavePtr);
-    ckfree((char *) slavePtr);
+    FreeSlave(slavePtr);
 }
diff --git a/tests/place.test b/tests/place.test
index 847047c..c2a4042 100644
--- a/tests/place.test
+++ b/tests/place.test
@@ -5,12 +5,15 @@
 # Copyright (c) 1998-1999 by Scriptics Corporation.
 # All rights reserved.
 #
-# RCS: @(#) $Id: place.test,v 1.10 2003/04/01 21:06:49 dgp Exp $
+# RCS: @(#) $Id: place.test,v 1.11 2004/09/16 18:01:20 pspjuth Exp $
 
 package require tcltest 2.1
 eval tcltest::configure $argv
 tcltest::loadTestedCommands
 
+# Used for constraining memory leak tests
+testConstraint memory [llength [info commands memory]]
+
 # XXX - This test file is woefully incomplete.  At present, only a
 # few of the features are tested.
 
@@ -380,6 +383,47 @@ test place-13.1 {test respect for internalborder} {
     set res
 } {196x188+2+10 177x186+5+7}
 
+test place-14.1 {memory leak testing} -setup {
+    proc getbytes {} {
+        set lines [split [memory info] "\n"]
+        lindex [lindex $lines 3] 3
+    }
+    # Repeat each body checking that memory does not increase
+    proc stress {args} {
+        set res {}
+        foreach body $args {
+            set end 0
+            for {set i 0} {$i < 5} {incr i} {
+                uplevel 1 $body
+                set tmp $end
+                set end [getbytes]
+            }
+            lappend res [expr {$end - $tmp}]
+        }
+        return $res
+    }
+} -constraints memory -body {
+    # Test all manners of forgetting a slave
+    frame .f
+    frame .f.f
+    stress {
+        place .f.f -x [expr {1 + 1}] -y [expr {2 + 2}]
+        place forget .f.f
+    } {
+        place .f.f -x [expr {1 + 1}] -y [expr {2 + 2}]
+        pack .f.f
+    } {
+        place .f.f -x [expr {1 + 1}] -y [expr {2 + 2}]
+        destroy .f
+        frame .f
+        frame .f.f
+    }
+} -result {0 0 0} -cleanup {
+    destroy .f
+    rename getbytes {}
+    rename stress {}
+}
+
 catch {destroy .t}
 
 # cleanup