domains: handle not being removed correctly

nodejs · Sep 19, 2011 · e1eee5b · e1eee5b
1 parent 517604b
commit e1eee5b
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 11 deletions.
diff --git a/lib/domains.js b/lib/domains.js
@@ -74,7 +74,6 @@ Domain.prototype.kill = function() {
   while (handle = this.handles.shift()) {
     console.error("shift");
     debug("kill handle", handle);
-    debugger;
 
     if (handle._onTimeout || handle.ontimeout) {
       // We may be calling domain.remove(handle) on a domain which is not our
@@ -166,7 +165,10 @@ exports.remove = function(handle) {
       }
 
       var i = handle.domain.handles.indexOf(handle);
-      handle.domain.handles.splice(i, 1);
+      if (i >= 0) {
+        handle.domain.handles.splice(i, 1);
+      }
+
       handle.domain = null;
     } else {
       assert.equal(currentDomain, defaultDomain);

diff --git a/lib/timers_uv.js b/lib/timers_uv.js
@@ -80,8 +80,8 @@ function insert(item, msecs) {
     list.ontimeout = function() {
       if (process.features.domains) {
         assert(this == list);
-        assert(list.domains);
-        assert(list.domains == require('domains').defaultDomain);
+        assert(list.domain);
+        assert(list.domain == require('domains').defaultDomain);
         assert(require('domains').getCurrent() ==
             require('domains').defaultDomain);
       }
@@ -109,8 +109,8 @@ function insert(item, msecs) {
               // it as its own handle.
               process.dispatch(first, "_onTimeout");
               assert(this == list);
-              assert(list.domains);
-              assert(list.domains == require('domains').defaultDomain);
+              assert(list.domain);
+              assert(list.domain == require('domains').defaultDomain);
               assert(require('domains').getCurrent() ==
                   require('domains').defaultDomain);
             } else {
@@ -247,7 +247,6 @@ exports.clearTimeout = function(timer) {
     if (timer instanceof Timer) {
       closeTimer(timer); // for after === 0
     } else {
-      debugger;
       exports.unenroll(timer);
     }
   }

diff --git a/test/simple/test-domains2.js b/test/simple/test-domains2.js
@@ -21,15 +21,15 @@
 
 var common = require('../common');
 var assert = require('assert');
-var domain = require('domain');
+var domains = require('domains');
 
 var timerCallbacks = 0;
 
 setTimeout(function() {
   console.error("timer 1");
   timerCallbacks++;
 
-  var d1 = domain.create(null, function() {
+  var d1 = domains.create(null, function() {
     console.error("d1 created");
 
     setTimeout(function() {
@@ -53,21 +53,22 @@ setTimeout(function() {
   timerCallbacks++;
   console.error("timer 4");
 
-  var d2 = domain.create(null, function() {
+  var d2 = domains.create(null, function() {
     console.error("d2 created");
 
     var t1 = setTimeout(function() {
       assert.ok(false, "should not get here");
     }, 1);
     assert.ok(t1.domain);
     assert.equal(t1.domain, d2);
-    assert.equal(t1.domain.id, require('domain').getCurrent().id);
+    assert.equal(t1.domain.id, require('domains').getCurrent().id);
 
     var t2 = setTimeout(function() {
       assert.ok(false, "should not get here");
     }, 1);
     assert.equal(t2.domain, d2);
 
+    // Both t1 and t2 will be canceled by the following kill.
     this.kill();
   });
 }, 1);