Tcl package Thread source code

rkeene stumbled across this segfault:

    package require Thread

    set t [thread::create]

    set resultvar() {}

    trace add variable resultvar() write {
        unset -nocomplain resultvar()
        puts unset
        list }

    thread::send -async $t {
        error "oh no"
    } resultvar()

    vwait forever

What seems to be going on is ThreadClbkSetVar is a bit loose with refcounts on the error path.
http://wiki.tcl.tk/14880 links to past discussion that is related.

The following patch fixes it on trunk [b76b97e8909] and doesn't generate any test failures.


--- generic/threadCmd.c
+++ generic/threadCmd.c
@@ -1637,18 +1637,21 @@
      * Get the result of the posted command.
      * We will use it to fill-in the result variable.
      */
 
     valObj = Tcl_NewStringObj(resultPtr->result, -1);
+
     if (resultPtr->result != threadEmptyResult) {
         ckfree(resultPtr->result);
     }
 
     /*
      * Set the result variable
      */
-
+    if (resultPtr->code == TCL_ERROR) {
+        Tcl_IncrRefCount(valObj);
+    }
     if (Tcl_SetVar2Ex(interp, var, NULL, valObj,
                       TCL_GLOBAL_ONLY | TCL_LEAVE_ERR_MSG) == NULL) {
         return TCL_ERROR;
     }
 
@@ -1666,10 +1669,11 @@
             var = "errorInfo";
             Tcl_SetVar2Ex(interp, var, NULL, Tcl_NewStringObj(resultPtr->errorInfo, -1), TCL_GLOBAL_ONLY);
             ckfree((char*)resultPtr->errorInfo);
         }
         Tcl_SetObjResult(interp, valObj);
+        Tcl_DecrRefCount(valObj);
         Tcl_BackgroundError(interp);
     }
 
     return TCL_OK;
 }

Committed in [f4c95731c0056f6d]

Wouldn't you leak valObj if Tcl_SetVar2Ex fails? How about the patch attached?