From: SourceForge.net <no...@so...> - 2005-10-22 22:42:39
|
Bugs item #1334947, was opened at 2005-10-22 12:25 Message generated for change (Comment added) made by msofer You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1334947&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 07. Variables Group: current: 8.4.11 Status: Open Resolution: None Priority: 9 Submitted By: miguel sofer (msofer) Assigned to: Don Porter (dgp) Summary: refCounts on PtrSetVar failure Initial Comment: The refCount management of TclPtrSetVar, and also its different "front ends" in tclVar.c, presents one particular problem which was revealed by [Bug 1191369] (both 8.4 and 8.5 are concerned). The problem arises when the newValuePtr obj has refCount 0: (1) if setting the new value succeeds as requested, newValuePtr gets its refCount incremented to reflect the new reference (2) if setting the new value fails early (eg, trying to set an array to a scalar value), newValuePtr's refCount is not touched (3) If there is a write trace that does not allow the value to be set as requested, newValuePtr's refCount will be incremented/decremented, resulting in it being freed. This renders the caller's refCount management rather critical, as reflected in 1191369. There are other spots in the core where a similar bug is present. Example: Tcl_AddObjErrorInfo would leak an obj if errorCode or errorInfo redefined to be arrays (I know, well deserved); Tcl_Main would leak if "argc" was an array. Worse, witness (in a MEM_DEBUG HEAD): % trace add variable a write {return -code error "RO var";#} % regexp qw qwerty -> a couldn't set variable "a" % set a file = ../generic/tclExecute.c, line = 2099 Trying to increment refCount of previously disposed object. There are two possible fixes: (a) fix all callers so that they incr before calling, decr on return. Add a warning in the docs. (b) fix TclPtrSetVar so that it does incr/decr for all paths where it fails to set the value as requested, allowing the callers to delegate the refCount management to TclPtrSetVar. The callers still have to be fixed to not touch that refCount. I have a preference for (b); assigning to dgp as a request for opinion. Please comment and assign back. ---------------------------------------------------------------------- >Comment By: miguel sofer (msofer) Date: 2005-10-22 19:42 Message: Logged In: YES user_id=148712 Clarifying my thoughts: the real "bug" is that some failure paths do not clear 0-ref objs, some do not. This makes it imperative for callers to manage the refCount explicitly. If a new obj is created by the caller and used as new value without incrementing its refCount, the handling of error returns is impossible to get right:either you DecrRefCount and crash on errors caused by traces, or you don't and leak the obj on other errors. My preference for alternative (b) is that it allows the fire-and-forget technique: just call e.g. Tcl_ObjSetVar2(interp, part1Ptr, part2Ptr, Tcl_NewObj(), flags) and forget all about that object's fate in the knowledge that it will be disposed of proeprly (as is currently done in error by eg Tcl_AddObjErrorInfo). Maybe a similar policy would make sense for other interfaces, eg Tcl_ListObjAppendElement? ---------------------------------------------------------------------- Comment By: miguel sofer (msofer) Date: 2005-10-22 12:42 Message: Logged In: YES user_id=148712 Better crash example: % trace add variable a write {set a 1; return -code error "RO var";#} % regexp qw qwerty -> a file = ../generic/tclCmdMZ.c, line = 370 Trying to decrement refCount of previously disposed object. ---------------------------------------------------------------------- Comment By: miguel sofer (msofer) Date: 2005-10-22 12:30 Message: Logged In: YES user_id=148712 correction to (b): either not touch it, or else incr/decr on all paths. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1334947&group_id=10894 |