Make evalcx work like it's supposed to.

1. Move the context->Enter() call so that the global obj is available for writing.
2. On success, copy the modified global out to the sandbox object.
3. Don't copy functions in either direction.  They have scope and closures, and make for craziness when trying to keep contexts separate.
4. Only do the ->ToObject->Clone() on objects, so that simple values stay simple.
5. Update the test so that it tests all this stuff.

src/node.cc

@@ -862,6 +862,9 @@ Handle<Value> EvalCX(const Arguments& args) {
   // Create the new context
   Persistent<Context> context = Context::New();
 
+  // Enter and compile script
+  context->Enter();
+
   // Copy objects from global context, to our brand new context
   Handle<Array> keys = sandbox->GetPropertyNames();
 
@@ -869,11 +872,12 @@ Handle<Value> EvalCX(const Arguments& args) {
   for (i = 0; i < keys->Length(); i++) {
     Handle<String> key = keys->Get(Integer::New(i))->ToString();
     Handle<Value> value = sandbox->Get(key);
-    context->Global()->Set(key, value->ToObject()->Clone());
+    if (value->IsFunction()) continue;
+    if (value->IsObject()) {
+      value = value->ToObject()->Clone();
+    }
+    context->Global()->Set(key, value);
   }
-
-  // Enter and compile script
-  context->Enter();
 
   // Catch errors
   TryCatch try_catch;
@@ -887,6 +891,18 @@ Handle<Value> EvalCX(const Arguments& args) {
     result = script->Run();
     if (result.IsEmpty()) {
       result = ThrowException(try_catch.Exception());
+    } else {
+      // success! copy changes back onto the sandbox object.
+      keys = context->Global()->GetPropertyNames();
+      for (i = 0; i < keys->Length(); i++) {
+        Handle<String> key = keys->Get(Integer::New(i))->ToString();
+        Handle<Value> value = context->Global()->Get(key);
+        if (value->IsFunction()) continue;
+        if (value->IsObject()) {
+          value = value->ToObject()->Clone();
+        }
+        sandbox->Set(key, value);
+      }
     }
   }
 

test/simple/test-eval-cx.js

@@ -14,14 +14,19 @@ process.evalcx('hello = 2');
 assert.equal(5, hello);
 
 
-code = "foo = 1; bar = 2;";
-foo = 2;
-obj = { foo : 0 };
-process.evalcx(code, obj);
+code = "foo = 1;"
+     + "bar = 2;"
+     + "if (baz !== 3) throw new Error('test fail');"
+     + "quux.pwned = true;";
 
-/* TODO?
+foo = 2;
+var quux = { pwned : false };
+obj = { foo : 0, baz : 3, quux : quux };
+var baz = process.evalcx(code, obj);
 assert.equal(1, obj.foo);
 assert.equal(2, obj.bar);
-*/
+assert.equal(obj.quux.pwned, true);
+assert.equal(quux.pwned, false);
+assert.notEqual(quux, obj.quux);
 
 assert.equal(2, foo);