Skip to content

Commit c5776ca

Browse files
authored
Fix potential leak in JSValue assignment (#7814)
1 parent 891a19c commit c5776ca

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

src/js/core/wrapped_value.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ namespace ccf::js::core
4646
{
4747
if (this != &other)
4848
{
49+
if ((ctx != nullptr) && (JS_VALUE_GET_TAG(val) != JS_TAG_MODULE))
50+
{
51+
JS_FreeValue(ctx, val);
52+
}
4953
ctx = other.ctx;
5054
val = JS_DupValue(ctx, other.val);
5155
}

src/js/test/js.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,55 @@ foo.bar.baz;
538538
}
539539
}
540540

541+
static int get_ref_count(JSValue v)
542+
{
543+
REQUIRE(JS_VALUE_HAS_REF_COUNT(v));
544+
auto* p = (JSRefCountHeader*)JS_VALUE_GET_PTR(v);
545+
return p->ref_count;
546+
}
547+
548+
TEST_CASE("JSWrappedValue copy assignment frees old value")
549+
{
550+
JSRuntime* rt = JS_NewRuntime();
551+
REQUIRE(rt != nullptr);
552+
JSContext* ctx = JS_NewContext(rt);
553+
REQUIRE(ctx != nullptr);
554+
555+
// Create two distinct JS objects (heap-allocated, so ref-counted)
556+
JSValue obj_a = JS_NewObject(ctx); // ref_count == 1
557+
JSValue obj_b = JS_NewObject(ctx); // ref_count == 1
558+
559+
// Keep raw copies so we can inspect ref counts after wrapping
560+
JSValue raw_a = JS_DupValue(ctx, obj_a); // ref_count(a) == 2
561+
JSValue raw_b = JS_DupValue(ctx, obj_b); // ref_count(b) == 2
562+
563+
{
564+
// Wrap both via the rvalue constructor (takes ownership, no dup)
565+
ccf::js::core::JSWrappedValue wa(ctx, std::move(obj_a));
566+
ccf::js::core::JSWrappedValue wb(ctx, std::move(obj_b));
567+
568+
// raw_a has ref 2 (raw_a + wa), raw_b has ref 2 (raw_b + wb)
569+
REQUIRE(get_ref_count(raw_a) == 2);
570+
REQUIRE(get_ref_count(raw_b) == 2);
571+
572+
// Copy-assign: wa = wb. This should free the old obj_a held by wa
573+
wa = wb;
574+
575+
// obj_a should have been freed by the assignment, leaving only raw_a
576+
REQUIRE(get_ref_count(raw_a) == 1);
577+
// obj_b should now be referenced by wa, wb, and raw_b
578+
REQUIRE(get_ref_count(raw_b) == 3);
579+
}
580+
// After both wrappers are destroyed, only our raw refs should remain
581+
REQUIRE(get_ref_count(raw_a) == 1);
582+
REQUIRE(get_ref_count(raw_b) == 1);
583+
584+
JS_FreeValue(ctx, raw_a);
585+
JS_FreeValue(ctx, raw_b);
586+
JS_FreeContext(ctx);
587+
JS_FreeRuntime(rt);
588+
}
589+
541590
int main(int argc, char** argv)
542591
{
543592
ccf::js::register_class_ids();

0 commit comments

Comments
 (0)