Tcl Source Code

View Ticket
Login
Ticket UUID: 1706140
Title: check TclCallVarTraces callers
Type: Bug Version: obsolete: 8.5a6
Submitter: dgp Created on: 2007-04-23 20:07:26
Subsystem: 07. Variables Assigned To: dgp
Priority: 7 High Severity:
Status: Closed Last Modified: 2007-05-31 01:21:06
Resolution: Fixed Closed By: dgp
    Closed on: 2007-05-30 18:21:06
Description:
One of the leaks reported in
1705778 got tracked down to
a TclCallVarTraces() call
filtering out the
TCL_INTERP_DESTROYED flag,
so that a trace procedure
didn't get signaled that it
was time to free memory.

Since it was wrong in one place,
and has apparently been wrong
as long as Tcl's been in CVS,
someone should give all the
callers a look over for similar
problems.
User Comments: dgp added on 2007-05-31 01:21:06:
Logged In: YES 
user_id=80530
Originator: YES


I think all remaining issues
raised here are now covered
in 1716730 and 1714505

dgp added on 2007-05-11 01:23:57:
Logged In: YES 
user_id=80530
Originator: YES

backport committed.

dgp added on 2007-05-08 02:45:05:
Logged In: YES 
user_id=80530
Originator: YES

fix committed to HEAD

keeping open to determine
what to backport.

dgp added on 2007-05-08 00:21:55:
Logged In: YES 
user_id=80530
Originator: YES


what's left to do before
committing?

msofer added on 2007-05-04 12:50:25:

File Added - 227788: 1706140-2.patch

Logged In: YES 
user_id=148712
Originator: NO

I was already playing with this ... attached patch implements the TCL_INTERP_DESTROYED changes described by dgp (minus the deprecation and doc changes).
File Added: 1706140-2.patch

dgp added on 2007-05-04 12:15:39:
Logged In: YES 
user_id=80530
Originator: YES


There's a deeper brokenness
here that's becoming apparent.
The value of the TCL_INTERP_DESTROYED
flag is set once by the caller
of TclCallVarTraces.  Then TclCallVarTraces
might well call several traceProcs.
The first of these might delete the interp,
so for correct operations, TCL_INTERP_DESTROYED
ought to get set for subsequent traceProc
calls.  This implies TclCallVarTraces *should*
be setting this flag itself, as the only place
it can be done correctly, and the callers shouldn't
be burdened with making the attempt.

Looking a bit further it's clear that
doing this as a flag value passed to
a traceProc is basically pointless.
We also pass the interp to traceProc,
which can just as easily call Tcl_InterpDeleted()
for itself to check for the deleted interp
scenario.  We have to support the flag for
compatibility, but probably should stop
recommending that traceProcs refer to it
when they can more simply and reliably just
call Tcl_InterpDeleted() for themselves.

Ok, did some archeology and the explanation
is that TCL_INTERP_DESTROYED came first.
It's there in Tcl 7.4 and who knows how long
before that.  Tcl_InterpDeleted() didn't show
up until Tcl 7.5 (3/22/96).

Well, it's well established
now, and is the better answer.  It's time
for the TCL_INTERP_DESTROYED flag to just
fade away.

dgp added on 2007-05-04 05:24:21:

File Added - 227758: 1706140.patch

Logged In: YES 
user_id=80530
Originator: YES


This patch does lots of
cleanup and safety for the
flag values passed around
tclVar.c.  Might be further
improved by moving all setting
of the TCL_INTERP_DESTROYED flag
value into TclCallVarTraces, instead
of having all callers set it.

File Added: 1706140.patch

Attachments: