Fix object serialization (#340)

Use of ser_array_begin in two cases caused an off-by-one error in the
object back-reference count that is used to stop an object from being
encoded more than once (and thus maintains object identity, allowing
recursive structures.)

Author: bnoordhuis

ext/mini_racer_extension/mini_racer_extension.c

@@ -611,6 +611,13 @@ static int serialize1(Ser *s, VALUE refs, VALUE v)
     return 0;
 }
 
+// don't mix with ser_array_begin/ser_object_begin because
+// that will throw off the object reference count
+static int serialize(Ser *s, VALUE v)
+{
+    return serialize1(s, rb_hash_new(), v);
+}
+
 static struct timespec deadline_ms(int ms)
 {
     static const int64_t ns_per_sec = 1000*1000*1000;
@@ -859,18 +866,11 @@ static void *rendezvous_callback(void *arg)
         goto fail;
     }
     ser_init1(&s, 'c'); // callback reply
-    ser_array_begin(&s, 2);
-    // either [result, undefined] or [undefined, err]
-    if (exc)
-        ser_undefined(&s);
-    if (serialize1(&s, rb_hash_new(), r)) { // should not happen
+    if (serialize(&s, r)) { // should not happen
         c->exception = rb_exc_new_cstr(internal_error, s.err);
         ser_reset(&s);
         goto fail;
     }
-    if (!exc)
-        ser_undefined(&s);
-    ser_array_end(&s, 2);
 out:
     buf_move(&s.b, a->req);
     return NULL;
@@ -1202,25 +1202,21 @@ static VALUE context_stop(VALUE self)
 
 static VALUE context_call(int argc, VALUE *argv, VALUE self)
 {
-    VALUE a, e, h;
+    VALUE name, args;
+    VALUE a, e;
     Context *c;
-    int i;
     Ser s;
 
     TypedData_Get_Struct(self, Context, &context_type, c);
-    rb_scan_args(argc, argv, "1*", &a, &e);
-    Check_Type(a, T_STRING);
+    rb_scan_args(argc, argv, "1*", &name, &args);
+    Check_Type(name, T_STRING);
+    rb_ary_unshift(args, name);
     // request is (C)all, [name, args...] array
     ser_init1(&s, 'C');
-    ser_array_begin(&s, argc);
-    h = rb_hash_new();
-    for (i = 0; i < argc; i++) {
-        if (serialize1(&s, h, argv[i])) {
-            ser_reset(&s);
-            rb_raise(runtime_error, "Context.call: %s", s.err);
-        }
+    if (serialize(&s, args)) {
+        ser_reset(&s);
+        rb_raise(runtime_error, "Context.call: %s", s.err);
     }
-    ser_array_end(&s, argc);
     // response is [result, err] array
     a = rendezvous(c, &s.b); // takes ownership of |s.b|
     e = rb_ary_pop(a);

ext/mini_racer_extension/mini_racer_v8.cc

@@ -378,18 +378,7 @@ void v8_api_callback(const v8::FunctionCallbackInfo<v8::Value>& info)
     des.ReadHeader(st.context).Check();
     v8::Local<v8::Value> result;
     if (!des.ReadValue(st.context).ToLocal(&result)) return; // exception pending
-    v8::Local<v8::Object> response; // [result, err]
-    if (!result->ToObject(st.context).ToLocal(&response)) return;
-    v8::Local<v8::Value> err;
-    if (!response->Get(st.context, 1).ToLocal(&err)) return;
-    if (err->IsUndefined()) {
-        if (!response->Get(st.context, 0).ToLocal(&result)) return;
-        info.GetReturnValue().Set(result);
-    } else {
-        v8::Local<v8::String> message;
-        if (!err->ToString(st.context).ToLocal(&message)) return;
-        st.isolate->ThrowException(v8::Exception::Error(message));
-    }
+    info.GetReturnValue().Set(result);
 }
 
 // response is err or empty string

test/mini_racer_test.rb

@@ -1126,4 +1126,13 @@ def test_string_encoding
       assert_equal Encoding::UTF_16LE, context.eval("'ok\\uD800'").encoding
     end
   end
+
+  def test_object_ref
+    context = MiniRacer::Context.new
+    context.eval("function f(o) { return o }")
+    expected = {}
+    expected["a"] = expected["b"] = {"x" => 42}
+    actual = context.call("f", expected)
+    assert_equal actual, expected
+  end
 end