[Tcl-bugs] [ tcl-Bugs-1334947 ] refCounts on PtrSetVar failure

[SourceForge.net <no...@so...>]

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