Merge pull request #36 from metamatt/preemption-fix

Call callback directly to avoid unwanted preemption.

src/weakref.cc

@@ -136,7 +136,8 @@ NAN_PROPERTY_ENUMERATOR(WeakPropertyEnumerator) {
 }
 
 /**
- * Weakref callback function. Invokes the "global" callback function.
+ * Weakref callback function. Invokes the "global" callback function,
+ * which emits the _CB event on the per-object EventEmitter.
  */
 
 NAN_WEAK_CALLBACK(TargetCallback) {
@@ -150,7 +151,11 @@ NAN_WEAK_CALLBACK(TargetCallback) {
     NanNew<Object>(cont->cbinfo->persistent),
     NanNew<Object>(cont->emitter)
   };
-  globalCallback->Call(2, argv);
+  // Invoke callback directly, not via NanCallback->Call() which uses
+  // node::MakeCallback() which calls into process._tickCallback()
+  // too. Those other callbacks are not safe to run from here.
+  v8::Local<v8::Function> globalCallbackDirect = globalCallback->GetFunction();
+  globalCallbackDirect->Call(NanGetCurrentContext()->Global(), 2, argv);
 
   // clean everything up
   Local<Object> proxy = NanNew<Object>(cont->proxy);

test/callback.js

@@ -45,6 +45,29 @@ describe('weak()', function () {
       assert(called2)
     })
 
+    it('should preempt code for GC callback but not nextTick callbacks'
+    , function(done) {
+      var calledGcCallback = false
+      , calledTickCallback = false
+      weak({}, function() {
+        calledGcCallback = true
+      })
+
+      process.nextTick(function() {
+        calledTickCallback = true
+      });
+
+      assert(!calledGcCallback)
+      assert(!calledTickCallback)
+      gc()
+      assert(calledGcCallback)
+      assert(!calledTickCallback)
+      setTimeout(function() {
+        assert(calledTickCallback);
+        done();
+      }, 0)
+    })
+
   })
 })