From 49f30082dd3c2a494c7a7bca131dbd509953c961 Mon Sep 17 00:00:00 2001 From: andreas_kupries Date: Mon, 11 Aug 2008 19:01:54 +0000 Subject: * generic/tclProc.c (Tcl_ProcObjCmd): Fixed memory leak triggered * tests/proc.test: by procbody::test::proc. See [Bug 2043636]. Added a test case demonstrating the leak before the fix. Fixed a few spelling errors in test descriptions as well. --- ChangeLog | 7 +++++++ generic/tclProc.c | 28 ++++++++++++++++++++++++---- tests/proc.test | 36 +++++++++++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index ff762de..7a68a70 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2008-08-11 Andreas Kupries + + * generic/tclProc.c (Tcl_ProcObjCmd): Fixed memory leak triggered + * tests/proc.test: by procbody::test::proc. See [Bug 2043636]. + Added a test case demonstrating the leak before the fix. Fixed a + few spelling errors in test descriptions as well. + 2008-08-11 Don Porter * library/http/http.tcl: Bump http version to 2.7.1 to account diff --git a/generic/tclProc.c b/generic/tclProc.c index 2b7c5f7..bf67600 100644 --- a/generic/tclProc.c +++ b/generic/tclProc.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: tclProc.c,v 1.139.2.2 2008/07/25 20:30:47 andreas_kupries Exp $ + * RCS: @(#) $Id: tclProc.c,v 1.139.2.3 2008/08/11 19:01:59 andreas_kupries Exp $ */ #include "tclInt.h" @@ -245,6 +245,7 @@ Tcl_ProcObjCmd( if (contextPtr->line && (contextPtr->nline >= 4) && (contextPtr->line[3] >= 0)) { int isNew; + Tcl_HashEntry* hePtr; CmdFrame *cfPtr = (CmdFrame *) ckalloc(sizeof(CmdFrame)); cfPtr->level = -1; @@ -261,8 +262,26 @@ Tcl_ProcObjCmd( cfPtr->cmd.str.cmd = NULL; cfPtr->cmd.str.len = 0; - Tcl_SetHashValue(Tcl_CreateHashEntry(iPtr->linePBodyPtr, - (char *) procPtr, &isNew), cfPtr); + hePtr = Tcl_CreateHashEntry(iPtr->linePBodyPtr, (char *) procPtr, &isNew); + if (!isNew) { + /* + * Get the old command frame and release it. See also + * TclProcCleanupProc in this file. Currently it seems as + * if only the procbodytest::proc command of the testsuite + * is able to trigger this situation. + */ + + CmdFrame* cfOldPtr = (CmdFrame *) Tcl_GetHashValue(hePtr); + + if (cfOldPtr->type == TCL_LOCATION_SOURCE) { + Tcl_DecrRefCount(cfOldPtr->data.eval.path); + cfOldPtr->data.eval.path = NULL; + } + ckfree((char *) cfOldPtr->line); + cfOldPtr->line = NULL; + ckfree((char *) cfOldPtr); + } + Tcl_SetHashValue(hePtr, cfPtr); } /* @@ -2162,7 +2181,8 @@ TclProcCleanupProc( /* * TIP #280: Release the location data associated with this Proc * structure, if any. The interpreter may not exist (For example for - * procbody structures created by tbcload. + * procbody structures created by tbcload. See also Tcl_ProcObjCmd(), when + * the same ProcPtr is overwritten with a new CmdFrame. */ if (!iPtr) { diff --git a/tests/proc.test b/tests/proc.test index 8fdc159..bb50979 100644 --- a/tests/proc.test +++ b/tests/proc.test @@ -13,7 +13,7 @@ # See the file "license.terms" for information on usage and redistribution # of this file, and for a DISCLAIMER OF ALL WARRANTIES. # -# RCS: @(#) $Id: proc.test,v 1.19 2006/11/03 00:34:53 hobbs Exp $ +# RCS: @(#) $Id: proc.test,v 1.19.4.1 2008/08/11 19:02:02 andreas_kupries Exp $ if {[lsearch [namespace children] ::tcltest] == -1} { package require tcltest @@ -26,6 +26,8 @@ if {[catch {package require procbodytest}]} { testConstraint procbodytest 1 } +testConstraint memory [llength [info commands memory]] + catch {namespace delete {*}[namespace children :: test_ns_*]} catch {rename p ""} catch {rename {} ""} @@ -233,7 +235,7 @@ test proc-4.3 {TclCreateProc, procbody obj, too many args} procbodytest { catch {rename t ""} set result } {procedure "t": arg list contains 3 entries, precompiled header expects 1} -test proc-4.4 {TclCreateProc, procbody obj, inconsitent arg name} procbodytest { +test proc-4.4 {TclCreateProc, procbody obj, inconsistent arg name} procbodytest { catch { proc p {x y z} { set v [join [list $x $y $z]] @@ -249,7 +251,7 @@ test proc-4.4 {TclCreateProc, procbody obj, inconsitent arg name} procbodytest { catch {rename t ""} set result } {procedure "t": formal parameter 1 is inconsistent with precompiled body} -test proc-4.5 {TclCreateProc, procbody obj, inconsitent arg default type} procbodytest { +test proc-4.5 {TclCreateProc, procbody obj, inconsistent arg default type} procbodytest { catch { proc p {x y {z Z}} { set v [join [list $x $y $z]] @@ -265,7 +267,7 @@ test proc-4.5 {TclCreateProc, procbody obj, inconsitent arg default type} procbo catch {rename t ""} set result } {procedure "t": formal parameter 2 is inconsistent with precompiled body} -test proc-4.6 {TclCreateProc, procbody obj, inconsitent arg default type} procbodytest { +test proc-4.6 {TclCreateProc, procbody obj, inconsistent arg default type} procbodytest { catch { proc p {x y z} { set v [join [list $x $y $z]] @@ -281,7 +283,7 @@ test proc-4.6 {TclCreateProc, procbody obj, inconsitent arg default type} procbo catch {rename t ""} set result } {procedure "t": formal parameter 2 is inconsistent with precompiled body} -test proc-4.7 {TclCreateProc, procbody obj, inconsitent arg default value} procbodytest { +test proc-4.7 {TclCreateProc, procbody obj, inconsistent arg default value} procbodytest { catch { proc p {x y {z Z}} { set v [join [list $x $y $z]] @@ -297,6 +299,30 @@ test proc-4.7 {TclCreateProc, procbody obj, inconsitent arg default value} procb catch {rename t ""} set result } {procedure "t": formal parameter "z" has default value inconsistent with precompiled body} +test proc-4.8 {TclCreateProc, procbody obj, no leak on multiple iterations} -setup { + proc getbytes {} { + set lines [split [memory info] "\n"] + lindex $lines 3 3 + } + proc px x { + set y [string tolower $x] + return "$x:$y" + } + px x +} -constraints {procbodytest memory} -body { + + set end [getbytes] + for {set i 0} {$i < 5} {incr i} { + + procbodytest::proc tx x px + + set tmp $end + set end [getbytes] + } + set leakedBytes [expr {$end - $tmp}] +} -cleanup { + rename getbytes {} +} -result 0 test proc-5.1 {Bytecompiling noop; test for correct argument substitution} { proc p args {} ; # this will be bytecompiled into t -- cgit v0.12