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 |
