View Ticket
Not logged in
Bounty program for improvements to Tcl and certain Tcl packages.
Ticket UUID: 84be1b5a73e57faf8b212e32dd275081c1ec0512
Title: segfault with -async and result trace
Type: Bug Version: trunk
Submitter: anonymous Created on: 2016-04-16 12:32:01
Subsystem: 49. Threading Assigned To: nobody
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2016-05-17 10:11:58
Resolution: None Closed By: gahr
    Closed on: 2016-05-17 10:11:58
Description:
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;
 }
User Comments: gahr added on 2016-05-17 10:11:58:
Committed in [f4c95731c0056f6d]

gahr added on 2016-05-13 13:59:57:
Wouldn't you leak valObj if Tcl_SetVar2Ex fails? How about the patch attached?

Attachments: