In Quickjs what is proper way to capture a string in a struct

Question

I have struct like this:

struct EventState{
std::string type;
};

I set this structure in constrctor of a class like this

eventState->type=JS_ToCString(ctx, argv[0]);

This line seems to cause memoery loss issues, as my valgrind complains 6 bytes in 1 blocks are definitely lost in loss record 1 of 2, at this exact line.

In quickjs how do one capture this string for internal purpose, without memory issues.

Upon request here is the minimal, reporducible example:

#include <quickjs/quickjs.h>
#include <iostream>

JSClassID newClassId=0;

struct EventState{
    std::string type;
};

static JSValue js_print(JSContext *ctx, JSValueConst this_val,
                        int argc, JSValueConst *argv)
{
    auto str = JS_ToCString(ctx, argv[0]);
    std::cout << "msg: "<<str << std::endl;
    JS_FreeCString(ctx, str);
    return JS_UNDEFINED;
}

static JSValue Event_constructor(JSContext *ctx, JSValueConst new_target,int argc, JSValueConst *argv){
    JSValue prototype;
    JSValue result=JS_UNDEFINED;
    auto id = newClassId;
    
    prototype = JS_GetPropertyStr(ctx,new_target,"prototype");

    if(JS_IsException(result)){
        return JS_EXCEPTION;
    }

    if(argc==0 || !JS_IsString(argv[0])){
        return JS_ThrowTypeError(ctx, "%s","Invalid event type");
    }
    
    result= JS_NewObjectProtoClass(ctx,prototype,id);
    JS_FreeValue(ctx, prototype);
    
    //set opaque if internal state needs to be maintained
    auto eventState = (EventState*)js_mallocz(ctx, sizeof(EventState));
    auto t = JS_ToCString(ctx, argv[0]);
    eventState->type=t;
    JS_FreeCString(ctx, t);
    JS_SetOpaque(result, eventState);

    return result;
}

static JSValue set_readonly(JSContext *ctx, JSValueConst this_val, JSValue val,int magic){
    return JS_UNDEFINED;
}
static JSValue get_type_Event(JSContext *ctx, JSValueConst this_val,int magic){
    auto eventState = (EventState*)JS_GetOpaque(this_val, newClassId);
    return JS_NewString(ctx, &eventState->type[0]);
}

JSCFunctionListEntry Event_instanceMethods[1]={
    JS_CGETSET_MAGIC_DEF("type", get_type_Event, set_readonly,0),
};

void registerEventClass(JSContext *ctx){
    JSClassID id=0;
    newClassId = JS_NewClassID(&id);
    auto rt = JS_GetRuntime(ctx);
    char* className ="Event";

    JSClassDef  classDef;
    classDef.class_name=className;
    classDef.finalizer=[](JSRuntime* rt, JSValue val){
        auto s  = (EventState*)JS_GetOpaque(val, newClassId);
        if(s!=NULL){
            js_free_rt(rt, s);
        }
    };
    classDef.gc_mark=NULL;
    classDef.exotic=NULL;
    classDef.call=NULL;

    JS_NewClass(rt,newClassId,&classDef);
    
    JSValue prototype =JS_NewObject(ctx);
    //adding instance methods
    JS_SetPropertyFunctionList(ctx,prototype,Event_instanceMethods,1);

    auto new_class = JS_NewCFunction2(ctx,Event_constructor,className,1,JS_CFUNC_constructor,0);
    auto global_obj=JS_GetGlobalObject(ctx);
    JS_DefinePropertyValueStr(ctx, global_obj, className,
                        JS_DupValue(ctx, new_class),
                        JS_PROP_WRITABLE | JS_PROP_CONFIGURABLE);
    JS_SetConstructor(ctx, new_class, prototype);
    JS_SetClassProto(ctx,newClassId,prototype);

    JS_FreeValue(ctx, new_class);
    JS_FreeValue(ctx,global_obj);

}

int main(){
        JSRuntime *rt;
        JSContext *ctx;
        JSValue evalVal;

        rt = JS_NewRuntime();
        ctx = JS_NewContext(rt);
        
        registerEventClass(ctx);

        auto global_object = JS_GetGlobalObject(ctx);
        auto printF = JS_NewCFunction(ctx, js_print, "print", 1);
        JS_SetPropertyStr(ctx, global_object, "print", printF);
        
        evalVal = JS_Eval(ctx, "let e = new Event('test1');\nprint(e.type);", 42, "<input>", 0);
        
        JS_FreeValue(ctx,global_object);
        JS_FreeValue(ctx, evalVal);
        
        JS_FreeContext(ctx);
        JS_FreeRuntime(rt);
        return 0;
}

Well yeah. You need to call `JS_FreeCString` on the string returned by `JS_ToCString`. — Captain Obvlious, Jan 08 '23 at 04:04
Well I did that too. I first assigned JS_ToCString output to a temproary variable `t` and then assigned that variable to `eventState->type` and then called `JS_FreeCSring`. Same error happens! — Anurag Vohra, Jan 08 '23 at 04:12
If we can't see it in a [*minimal,reproducable example*](https://stackoverflow.com/help/minimal-reproducible-example) it didn't happen. — Captain Obvlious, Jan 08 '23 at 04:16
Haven't run it but just a quick glance and I see that `Event_constructor` returns early without freeing `prototype` and using `js_mallocz` to create `EventState` will not call the constructor of `EventState::type` which is then assigned to which is undefined behaviour. Anything returned by `JS_`* that needs to be freed can and should be stuffed into either `std::unique_ptr` or `std::shared_ptr` to avoid these problems. — Captain Obvlious, Jan 08 '23 at 17:07
@Captain Obvlious std::unique_ptr or std::shared_ptr? Sure, but ONLY if you're using a custom deleter when you construct the std::unique_ptr or std::shared_ptr object. Otherwise you must make SURE that JS_ToCString is using new (NOT new[], not malloc, not js_mallocz, or whatever!!) If you don't use a custom deleter you're going to have undefined behavior. — George, Jan 17 '23 at 18:32

score 0 · Accepted Answer · answered Jan 17 '23 at 03:22

Ok so you can't use std::String for storing the string generated by Quickjs it seems to create issues like this. As std::string are managed by C++ and the memory where this strings are created is in Heap of QuickJS runtime.

So instead your struct should be like this :

struct EventState{
    char* type;
};

And then you need to copy the type like this:

auto t = JS_ToCString(ctx, argv[0]);
eventState->type = strdup(t); //we will still need to free this
JS_FreeCString(ctx, t);

And later free this eventState->type memory using free in finalizer

classDef.finalizer=[](JSRuntime* rt, JSValue val){
auto s  = (EventState*)JS_GetOpaque(val, newClassId);
if(s!=NULL){
    free(s->type);
    js_free_rt(rt, s);
 }
};

Here is the complete minimal running code example, with no memory issue:

#include <cstdlib>
#include <quickjs/quickjs.h>
#include <iostream>
#include <cstring>

JSClassID newClassId=0;

struct EventState{
    char* type;
};

static JSValue js_print(JSContext *ctx, JSValueConst this_val,
                        int argc, JSValueConst *argv)
{
    auto str = JS_ToCString(ctx, argv[0]);
    std::cout << "msg: "<<str << std::endl;
    JS_FreeCString(ctx, str);
    return JS_UNDEFINED;
}

static JSValue Event_constructor(JSContext *ctx, JSValueConst new_target,int argc, JSValueConst *argv){
    JSValue prototype;
    JSValue result=JS_UNDEFINED;
    auto id = newClassId;
    
    prototype = JS_GetPropertyStr(ctx,new_target,"prototype");

    if(JS_IsException(result)){
        return JS_EXCEPTION;
    }

    if(argc==0 || !JS_IsString(argv[0])){
        return JS_ThrowTypeError(ctx, "%s","Invalid event type");
    }
    
    result= JS_NewObjectProtoClass(ctx,prototype,id);
    JS_FreeValue(ctx, prototype);
    
    //set opaque if internal state needs to be maintained
    auto eventState = (EventState*)js_mallocz(ctx, sizeof(EventState));
    auto t = JS_ToCString(ctx, argv[0]);
    eventState->type = strdup(t);
    JS_FreeCString(ctx, t);
    JS_SetOpaque(result, eventState);

    return result;
}

static JSValue set_readonly(JSContext *ctx, JSValueConst this_val, JSValue val,int magic){
    return JS_UNDEFINED;
}
static JSValue get_type_Event(JSContext *ctx, JSValueConst this_val,int magic){
    auto eventState = (EventState*)JS_GetOpaque(this_val, newClassId);
    return JS_NewString(ctx, &eventState->type[0]);
}

JSCFunctionListEntry Event_instanceMethods[1]={
    JS_CGETSET_MAGIC_DEF("type", get_type_Event, set_readonly,0),
};

void registerEventClass(JSContext *ctx){
    JSClassID id=0;
    newClassId = JS_NewClassID(&id);
    auto rt = JS_GetRuntime(ctx);
    char* className ="Event";

    JSClassDef  classDef;
    classDef.class_name=className;
    classDef.finalizer=[](JSRuntime* rt, JSValue val){
        auto s  = (EventState*)JS_GetOpaque(val, newClassId);
        if(s!=NULL){
            free(s->type);
            js_free_rt(rt, s);
        }
    };
    classDef.gc_mark=NULL;
    classDef.exotic=NULL;
    classDef.call=NULL;

    JS_NewClass(rt,newClassId,&classDef);
    
    JSValue prototype =JS_NewObject(ctx);
    //adding instance methods
    JS_SetPropertyFunctionList(ctx,prototype,Event_instanceMethods,1);

    auto new_class = JS_NewCFunction2(ctx,Event_constructor,className,1,JS_CFUNC_constructor,0);
    auto global_obj=JS_GetGlobalObject(ctx);
    JS_DefinePropertyValueStr(ctx, global_obj, className,
                        JS_DupValue(ctx, new_class),
                        JS_PROP_WRITABLE | JS_PROP_CONFIGURABLE);
    JS_SetConstructor(ctx, new_class, prototype);
    JS_SetClassProto(ctx,newClassId,prototype);

    JS_FreeValue(ctx, new_class);
    JS_FreeValue(ctx,global_obj);

}

int main(){
        JSRuntime *rt;
        JSContext *ctx;
        JSValue evalVal;

        rt = JS_NewRuntime();
        ctx = JS_NewContext(rt);
        
        registerEventClass(ctx);

        auto global_object = JS_GetGlobalObject(ctx);
        auto printF = JS_NewCFunction(ctx, js_print, "print", 1);
        JS_SetPropertyStr(ctx, global_object, "print", printF);
        
        evalVal = JS_Eval(ctx, "let e = new Event('test1');\nprint(e.type);", 42, "<input>", 0);
        
        JS_FreeValue(ctx,global_object);
        JS_FreeValue(ctx, evalVal);
        
        JS_FreeContext(ctx);
        JS_FreeRuntime(rt);
        return 0;
}

What if argv[0] contains a NUL (`\0`) byte? Will the string be truncated? — pts, Jan 17 '23 at 03:28
Your understanding of `std::string` seems to be flawed. When you construct a `std::string` from `t` or assign `t` to a `std::string` it will make a copy of it just like `strdup` will. Using a `std::string` is fine just instantiate a `EventState` via `new` and change the finalizer to `delete static_cast(JS_GetOpaque(val, newClassId));` — Captain Obvlious, Jan 17 '23 at 20:37

In Quickjs what is proper way to capture a string in a struct

1 Answers1