Ticket Change Details
Not logged in
Overview

Artifact ID: 2ec9fe372f9172c4518ce75de50e754d4d1e5837713b0084da281390da493735
Ticket: 578155d5a19b348dc1a9fe96cc2c067a59326a89
Very rare bug (segfault) if set variable (with error case) using self-releasable object as new value
User & Date: sebres 2023-03-29 12:51:52
Changes

  1. icomment:
    Just for the record, here is simplest example to illustrate the issue:
    <code><pre>
    Tcl_Obj * CacheSetObj(Tcl_Obj *varName, Tcl_Obj *objPtr)
    {
      Tcl_ObjSetVar2(interp, cache, varName, objPtr, 0);
      return objPtr;
    }
    
    ...
    
    if ((objPtr = CacheGetObj(someName)) != NULL)
      return objPtr;
    return CacheSetObj(someName, SomethingToObtainObj(...));
    </pre></code>
    
    This is completely valid scenario and it'd even work almost always properly... unless <code>SomethingToObtainObj</code> returns unreferenced object and cache doesn't exists for some reason, because then after failed assignment the object gets deleted.
    
    Similar scenarios (like try to set to var1, if failed to var2/dict/whatever) are also pretty imaginable.
    As well as vise versa situation (unwillingness to use the object after set variable): necessity to free the object in error case (but conditionally) makes it hardly possible too, because one doesn't really know the state of object (whether it already removed or not).
    
    And again an increment and decrement around is not really a solution here, because:
    <ul>
    <li>it is totally unneeded action</li>
    <li>it is not obvious for the caller (basically the matter of responsibility of API functions and solves an inconsistency introduced in Tcl API)<li>
    <li>it cannot be seriously considered as mandatory merely on the grounds that the "fix" [510663a99e3a096bb7bab7314eb59fc805335318] becomes totally unnecessary (if one assumes only to use <code>Tcl_ObjSetVar2</code> with objects with refCount > 0)</li>.
    </ul>
    
    Rather Tcl needs some new (may be internal) function working with such a decrement, or a flag (like <code>TCL_OWN_OBJREF</code>) allowing the decrement in error case.
    
    And inconsistent "fix" [510663a99e3a096bb7bab7314eb59fc805335318] of <code>TclPtrSetVar</code> with <code>earlyError</code> block must be definitely reverted, merely for the reason that <b>a potential for an unlikely leak is many times better than an unlikely heisenbug (double free, usage after free, reference error etc)</b>, especially if a leak, that may arise for habitude reasons, occurs only in error case.
    
  2. login: "sebres"
  3. mimetype: "text/x-fossil-wiki"