Tcl Source Code

View Ticket
Login
Ticket UUID: 693564
Title: tcmdPtr leaked in rename and execution traces
Type: Bug Version: obsolete: 8.4.1.1
Submitter: msofer Created on: 2003-02-26 11:52:27
Subsystem: 45. Parsing and Eval Assigned To: hobbs
Priority: 8 Severity:
Status: Closed Last Modified: 2003-02-27 08:05:03
Resolution: Fixed Closed By: hobbs
    Closed on: 2003-02-27 01:05:03
Description:
The tcmdPtr structures alloced by rename and execution
traces are leaked. To see this, define the script
/tmp/trace1.tcl as

   proc foo {} {}
   trace add command foo rename traceCommand
   trace add execution foo enter traceCommand
   rename foo {}
   proc exit args {}

(note that the last line is just for insuring a proper
cleanup of the main interpreter, it has no impact on
the effects discussed here).

Now run that script through valgrind and see the
reported leaks:

[mig@mini unix]$ /opt/valgrind/bin/valgrind
--leak-check=yes --num-callers=10
--leak-resolution=high ./tclsh /tmp/trace1.tcl     
==29788== valgrind-1.0.0, a memory error detector for
x86 GNU/Linux.
==29788== Copyright (C) 2000-2002, and GNU GPL'd, by
Julian Seward.
==29788== Estimated CPU clock rate is 598 MHz
==29788== For more details, rerun with: -v
==29788== 
==29788== 
==29788== ERROR SUMMARY: 0 errors from 0 contexts
(suppressed: 0 from 0)
==29788== malloc/free: in use at exit: 90 bytes in 2
blocks.
==29788== malloc/free: 2277 allocs, 2275 frees, 185733
bytes allocated.
==29788== For counts of detected errors, rerun with: -v
==29788== searching for pointers to 2 not-freed blocks.
==29788== checked 4591484 bytes.
==29788== 
==29788== definitely lost: 90 bytes in 2 blocks.
==29788== possibly lost:   0 bytes in 0 blocks.
==29788== still reachable: 0 bytes in 0 blocks.
==29788== 
==29788== 45 bytes in 1 blocks are definitely lost in
loss record 1 of 2
==29788==    at 0x400467C4: malloc (vg_clientfuncs.c:100)
==29788==    by 0x80EDB61: TclpAlloc
(./../generic/tclAlloc.c:679)
==29788==    by 0x8072FF1: Tcl_Alloc
(./../generic/tclCkalloc.c:1002)
==29788==    by 0x8081E63: TclTraceExecutionObjCmd
(./../generic/tclCmdMZ.c:3233)
==29788==    by 0x8081786: Tcl_TraceObjCmd
(./../generic/tclCmdMZ.c:2980)
==29788==    by 0x806D9B1: TclEvalObjvInternal
(./../generic/tclBasic.c:3081)
==29788==    by 0x806E5D3: Tcl_EvalEx
(./../generic/tclBasic.c:3670)
==29788==    by 0x80BD37B: Tcl_FSEvalFile
(./../generic/tclIOUtil.c:1421)
==29788==    by 0x8054C1D: Tcl_Main
(./../generic/tclMain.c:292)
==29788==    by 0x80547EB: main (./../unix/tclAppInit.c:90)
==29788== 
==29788== 45 bytes in 1 blocks are definitely lost in
loss record 2 of 2
==29788==    at 0x400467C4: malloc (vg_clientfuncs.c:100)
==29788==    by 0x80EDB61: TclpAlloc
(./../generic/tclAlloc.c:679)
==29788==    by 0x8072FF1: Tcl_Alloc
(./../generic/tclCkalloc.c:1002)
==29788==    by 0x808244B: TclTraceCommandObjCmd
(./../generic/tclCmdMZ.c:3457)
==29788==    by 0x8081786: Tcl_TraceObjCmd
(./../generic/tclCmdMZ.c:2980)
==29788==    by 0x806D9B1: TclEvalObjvInternal
(./../generic/tclBasic.c:3081)
==29788==    by 0x806E5D3: Tcl_EvalEx
(./../generic/tclBasic.c:3670)
==29788==    by 0x80BD37B: Tcl_FSEvalFile
(./../generic/tclIOUtil.c:1421)
==29788==    by 0x8054C1D: Tcl_Main
(./../generic/tclMain.c:292)
==29788==    by 0x80547EB: main (./../unix/tclAppInit.c:90)
==29788== 
==29788== LEAK SUMMARY:
==29788==    definitely lost: 90 bytes in 2 blocks.
==29788==    possibly lost:   0 bytes in 0 blocks.
==29788==    still reachable: 0 bytes in 0 blocks.
==29788== Reachable blocks (those to which a pointer
was found) are not shown.
==29788== To see them, rerun with: --show-reachable=yes
==29788==
User Comments: hobbs added on 2003-02-27 08:05:03:
Logged In: YES 
user_id=72656

Applied for 8.4.2.  It passes all tests across platforms, and 
corrects the broken one in the mem_debug case.

msofer added on 2003-02-27 00:05:42:

File Added - 43531: trace.diff

Logged In: YES 
user_id=148712

OK, that seems to work: tcltest OK, no leaks in
tests/trace.test.

Enclosing a patch against HEAD, assigning to the release
manager to determine if it should be committed for (frozen)
8.4.2

vincentdarley added on 2003-02-26 23:14:54:
Logged In: YES 
user_id=32170

Thinking further, remove that "refCount--" from the 
else block so it happens whether flags is set to zero or 
not. I think both paths are safe.

msofer added on 2003-02-26 20:59:01:
Logged In: YES 
user_id=148712

Yes, but :(

That fixes indeed those two leaks, but due to my incomplete
analysis/guess, some other leaks still remain.

In trace-25 (tests/trace.test) tests 1-3 and 8-11 leak a
tcmdPtr each; the valgrind report is

 ==31189== definitely lost: 336 bytes in 7 blocks.
==31189== possibly lost:   0 bytes in 0 blocks.
==31189== still reachable: 0 bytes in 0 blocks.
==31189== 
==31189== 336 bytes in 7 blocks are definitely lost in loss
record 1 of 1
==31189==    at 0x400467C4: malloc (vg_clientfuncs.c:100)
==31189==    by 0x80FC511: TclpAlloc
(./../generic/tclAlloc.c:679)
==31189==    by 0x806C211: Tcl_Alloc
(./../generic/tclCkalloc.c:1002)
==31189==    by 0x807B083: TclTraceExecutionObjCmd
(./../generic/tclCmdMZ.c:3233)
==31189==    by 0x807A9A6: Tcl_TraceObjCmd
(./../generic/tclCmdMZ.c:2980)
==31189==    by 0x8066BD1: TclEvalObjvInternal
(./../generic/tclBasic.c:3081)
==31189==    by 0x80677F3: Tcl_EvalEx
(./../generic/tclBasic.c:3670)
==31189==    by 0x8067EB8: Tcl_EvalObjEx
(./../generic/tclBasic.c:3989)
==31189==    by 0x80CF94E: Tcl_UplevelObjCmd
(./../generic/tclProc.c:674)
==31189==    by 0x8066BD1: TclEvalObjvInternal
(./../generic/tclBasic.c:3081)
==31189== 
==31189== LEAK SUMMARY:
==31189==    definitely lost: 336 bytes in 7 blocks.
==31189==    possibly lost:   0 bytes in 0 blocks.
==31189==    still reachable: 0 bytes in 0 blocks.

vincentdarley added on 2003-02-26 19:52:55:
Logged In: YES 
user_id=32170

I think adding this 'else' block around line 4083 of 
tclCmdMZ.c will fix this:

if (tcmdPtr->flags & 
TCL_TRACE_EXEC_IN_PROGRESS) {
    /* Postpone deletion, until exec trace 
returns */
    tcmdPtr->flags = 0;
} else {
    /* 
     * Decrement the refCount since the 
command which held our
     * reference (ever since we were created) 
has just gone away
     */
    tcmdPtr->refCount--;
}

Attachments: