Retain trace-proxy callback (fix Hermes use-after-free)

valdi/src/valdi/runtime/JavaScript/JavaScriptRuntime.cpp

@@ -176,7 +176,7 @@ class JavaScriptRuntimeTraceProxyCallable : public JSFunction {
                                         const JSValue& callback)
         : _traceName(traceName),
           _referenceInfo(ReferenceInfoBuilder().withObject(nameFromJSFunction(jsContext, callback)).build()),
-          _callback(callback) {}
+          _callback(JSValueRef::makeRetained(jsContext, callback)) {}
 
     ~JavaScriptRuntimeTraceProxyCallable() override = default;
 
@@ -186,13 +186,13 @@ class JavaScriptRuntimeTraceProxyCallable : public JSFunction {
 
     JSValueRef operator()(JSFunctionNativeCallContext& callContext) noexcept override {
         VALDI_TRACE(_traceName);
-        return callContext.getContext().callObjectAsFunction(_callback, callContext);
+        return callContext.getContext().callObjectAsFunction(_callback.get(), callContext);
     }
 
 private:
     StringBox _traceName;
     ReferenceInfo _referenceInfo;
-    JSValue _callback;
+    JSValueRef _callback;
 };
 
 class JSRuntimeNativeObjectsManagerImpl : public snap::valdi_core::JSRuntimeNativeObjectsManager {

valdi/test/integration/Runtime_tests.cpp

@@ -272,6 +272,40 @@ TEST_P(RuntimeFixture, exposesDefaultApiVersion) {
     ASSERT_EQ(0, evalResult.value().toInt());
 }
 
+TEST_P(RuntimeFixture, traceProxyRetainsCallbackAcrossGC) {
+    // Regression: JavaScriptRuntimeTraceProxyCallable used to hold its wrapped callback as a bare,
+    // non-owning JSValue. Under Hermes the callback's pooled value could be collected and its slot
+    // recycled while the proxy still referenced it, so invoking the proxy hit a dangling handle and
+    // threw "Value is not a function". The proxy must retain the callback for its own lifetime.
+    auto javaScriptRuntime = wrapper.runtime->getJavaScriptRuntime();
+
+    // makeTraceProxy is only bound when tracing is compiled in (kTracingEnabled); skip otherwise.
+    auto probe = javaScriptRuntime->evaluateScript(
+        makeShared<ByteBuffer>(std::string("return typeof runtime.makeTraceProxy === 'function' ? 1 : 0;"))
+            ->toBytesView(),
+        STRING_LITERAL("eval.js"));
+    ASSERT_TRUE(probe) << probe.description();
+    if (probe.value().toInt() != 1) {
+        GTEST_SKIP() << "tracing disabled in this build (runtime.makeTraceProxy not bound)";
+    }
+
+    // The closure has no JS reference other than the trace proxy itself.
+    std::string setupBody = "globalThis.__traceProxy = runtime.makeTraceProxy('regressionTag', () => 42); return 0;";
+    auto setupResult =
+        javaScriptRuntime->evaluateScript(makeShared<ByteBuffer>(setupBody)->toBytesView(), STRING_LITERAL("eval.js"));
+    ASSERT_TRUE(setupResult) << setupResult.description();
+
+    // Force GC: pre-fix the unrooted closure is collected and its slot recycled.
+    javaScriptRuntime->dispatchSynchronouslyOnJsThread([](auto& jsEntry) { jsEntry.jsContext.garbageCollect(); });
+
+    // Invoke through the proxy: it must still resolve to the original callback.
+    std::string callBody = "return __traceProxy();";
+    auto callResult =
+        javaScriptRuntime->evaluateScript(makeShared<ByteBuffer>(callBody)->toBytesView(), STRING_LITERAL("eval.js"));
+    ASSERT_TRUE(callResult) << callResult.description();
+    ASSERT_EQ(42, callResult.value().toInt());
+}
+
 TEST_P(RuntimeFixture, canLoadSimpleViewTree) {
     auto tree = wrapper.createViewNodeTreeAndContext("test", "BasicViewTree");