Lib using Tcl C API is crashing maybe due bad refCount usage

Question

I'm a beginner at Tcl C API and I'm trying to understand how to use it. I have this code that gets a tcl list from get_my_list proc and then I iterate over it to dispatch some info, in this case, info related to attributes A, B and C that I get with get_attr_info proc. When I run it in a very simple example that basically has only this code, everything works perfectly, but when I add this lib in a big tcl project, eventually it crashes. I suspect that it's due to bad usage of reference counting of tcl objects, I mean, their lifetimes. What could I be doing wrong in the example below? I'm using Tcl 8.6.

Tcl_Obj* Get_Info(Tcl_Interp *interp, const char* info, const char* attr) {
    char cmd[256];
    sprintf(cmd, "get_attr_info %s %s", info, attr);
    Tcl_Eval(interp, cmd);
    return Tcl_GetObjResult(interp);
}

static int Copy_Info(ClientData cdata, Tcl_Interp *interp, int objc, Tcl_Obj *const objv[]) {
    Tcl_Eval(interp, "get_my_list");
    Tcl_Obj *const my_list = Tcl_GetObjResult(interp);
    int my_list_size;
    Tcl_ListObjLength(interp, my_list, &my_list_size);
    Tcl_IncrRefCount(my_list);

    for (int i = 0; i < my_list_size; ++i) {
        Tcl_Obj* current_info_obj;
        Tcl_ListObjIndex(interp, my_list, i, &current_info_obj);
        const char* current_info_ctr = Tcl_GetStringFromObj(current_info_obj, NULL);

        /* getting info A */
        Tcl_Obj* info_a_obj = Get_Info(current_info_ctr, "A");
        const char* info_a = Tcl_GetStringFromObj(info_a_obj, NULL);
        Copy_Info_A(info_a);

        /* getting info B */
        Tcl_Obj* info_b_obj = Get_Info(current_info_ctr, "B");
        const char* info_b = Tcl_GetStringFromObj(info_b_obj, NULL);
        Copy_Info_B(info_b);

        /* getting info C */
        Tcl_Obj* info_c_obj = Get_Info(current_info_ctr, "C");
        const char* info_c = Tcl_GetStringFromObj(info_c_obj, NULL);
        Copy_Info_C(info_c);
    }

    Tcl_DecrRefCount(my_list);
    Tcl_FreeResult(interp);

    return TCL_OK;
}

Answer 1

The tricky bit is that you call Tcl_Eval() inside Get_Info(), which can do all sorts of things with reference count management of values that you're not holding your own reference to, including shimmering away the list representation of the value you're holding a reference to in my_list, which can pull the rug out from under its elements. In particular, the object referred to in current_info_obj must have its reference count incremented from after the Tcl_ListObjIndex() to the end of the loop body.

// ...
for (int i = 0; i < my_list_size; ++i) {
    Tcl_Obj* current_info_obj;
    Tcl_ListObjIndex(interp, my_list, i, &current_info_obj);
    // HOLD THE REFERENCE; IT OWNS THE current_info_ctr STRING FOR US
    Tcl_IncrRefCount(current_info_obj);
    const char* current_info_ctr = Tcl_GetStringFromObj(current_info_obj, NULL);

    /* getting info A */
    Tcl_Obj* info_a_obj = Get_Info(current_info_ctr, "A");
    const char* info_a = Tcl_GetStringFromObj(info_a_obj, NULL);
    Copy_Info_A(info_a);

    /* getting info B */
    Tcl_Obj* info_b_obj = Get_Info(current_info_ctr, "B");
    const char* info_b = Tcl_GetStringFromObj(info_b_obj, NULL);
    Copy_Info_B(info_b);

    /* getting info C */
    Tcl_Obj* info_c_obj = Get_Info(current_info_ctr, "C");
    const char* info_c = Tcl_GetStringFromObj(info_c_obj, NULL);
    Copy_Info_C(info_c);

    // RELEASE THE REFERENCE
    Tcl_DecrRefCount(current_info_obj);
}
// ...

---

Improving your code

You can also do a short-cut, and use Tcl_GetString(objPtr) in place of Tcl_GetStringFromObj(objPtr, NULL). You might put that inside Get_Info. You probably also ought to consider switching to using Tcl_EvalObjv(), as that's a faster API (it avoids a level of parsing), though a more complex one to use.

const char *Get_Info(Tcl_Interp *interp, Tcl_Obj* info, const char* attr) {
    Tcl_Obj *argv[3], *result;

    argv[0] = Tcl_NewStringObj("get_attr_info", -1); // cacheable
    argv[1] = info;
    argv[2] = Tcl_NewStringObj(attr, -1);
    Tcl_IncrRefCount(argv[0]);
    Tcl_IncrRefCount(argv[1]);
    Tcl_IncrRefCount(argv[2]);

    // It's very bad form to omit error handling
    if (Tcl_EvalObjv(interp, 3, argv, 0) != TCL_OK) {
        Tcl_Panic("problem: %s", Tcl_GetString(Tcl_GetObjResult(interp)));
    }
    result = Tcl_GetObjResult(interp);
    Tcl_DecrRefCount(argv[0]);
    Tcl_DecrRefCount(argv[1]);
    Tcl_DecrRefCount(argv[2]);
    return Tcl_GetString(result);
}

There are cases where it's possible to reduce the amount of reference count handling. Don't worry about doing that to start with! Better to definitely avoid crashes!

Tcl_EvalObjv is (effectively) what Tcl_Eval calls to actually dispatch a command after parsing in interactive mode, and it's what the bytecode engine calls to evaluate any non-inlined command. It's much more efficient than parsing a string into words.