Fix dgram invokes callback either sync or async.

Add callback argument to addMembership()/removeMembership(). Fixes #1456.
nodejs · Aug 21, 2011 · 09f753f · 09f753f
1 parent f5db3f1
commit 09f753f
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 39 deletions.
diff --git a/lib/dgram.js b/lib/dgram.js
@@ -62,6 +62,19 @@ function dnsLookup(type, hostname, callback) {
   });
 }
 
+// Make sure that the callback is invoked asynchronously.
+function makeAsync(callback) {
+  if (typeof callback !== 'function') {
+    return callback;
+  }
+  return function asyncCallback() {
+    var args = arguments;
+    process.nextTick(function() {
+      callback.apply(null, args);
+    });
+  };
+}
+
 function Socket(type, listener) {
   events.EventEmitter.call(this);
   var self = this;
@@ -138,7 +151,9 @@ Socket.prototype.bind = function() {
       // Not bind()ing a specific address. Use INADDR_ANY and OS will pick one.
       // The address can be found with server.address()
       binding.bind(self.fd, arguments[0]);
-      this.emit('listening');
+      process.nextTick(function() {
+        self.emit('listening');
+      });
     } else {
       // the first argument is the port, the second an address
       this.port = arguments[0];
@@ -205,26 +220,52 @@ Socket.prototype.setMulticastLoopback = function(arg) {
 };
 
 Socket.prototype.addMembership = function(multicastAddress,
-                                          multicastInterface) {
+                                          multicastInterface,
+                                          callback) {
   var self = this;
+
+  if (typeof multicastInterface === 'function') {
+    callback = multicastInterface;
+    multicastInterface = undefined;
+  }
+
   dnsLookup(this.type, multicastAddress, function(err, ip, addressFamily) {
     if (err) {  // DNS error
+      if (callback) {
+        callback(err);
+      }
       self.emit('error', err);
       return;
     }
-    binding.addMembership(self.fd, multicastAddress, multicastInterface);
+    binding.addMembership(self.fd, ip, multicastInterface);
+    if (callback) {
+      callback(null);
+    }
   });
 };
 
 Socket.prototype.dropMembership = function(multicastAddress,
-                                           multicastInterface) {
+                                           multicastInterface,
+                                           callback) {
   var self = this;
+
+  if (typeof multicastInterface === 'function') {
+    callback = multicastInterface;
+    multicastInterface = undefined;
+  }
+
   dnsLookup(this.type, multicastAddress, function(err, ip, addressFamily) {
     if (err) {  // DNS error
+      if (callback) {
+        callback(err);
+      }
       self.emit('error', err);
       return;
     }
-    binding.dropMembership(self.fd, multicastAddress, multicastInterface);
+    binding.dropMembership(self.fd, ip, multicastInterface);
+    if (callback) {
+      callback(null);
+    }
   });
 };
 
@@ -243,7 +284,8 @@ Socket.prototype.send = function(buffer, offset, length) {
                       'in the filesystem');
     }
 
-    self.sendto(buffer, offset, length, arguments[3], null, arguments[4]);
+    self.sendto(buffer, offset, length, arguments[3], null,
+                makeAsync(arguments[4]));
   } else if (this.type === 'udp4' || this.type === 'udp6') {
     // send(buffer, offset, length, port, address [, callback])
     if (typeof arguments[4] !== 'string') {
@@ -252,7 +294,7 @@ Socket.prototype.send = function(buffer, offset, length) {
 
     if (binding.isIP(arguments[4])) {
       self.sendto(arguments[0], arguments[1], arguments[2], arguments[3],
-                  arguments[4], arguments[5]);
+                  arguments[4], makeAsync(arguments[5]));
     } else {
       var port = arguments[3],
           callback = arguments[5];

diff --git a/test/simple/test-dgram-multicast.js b/test/simple/test-dgram-multicast.js
@@ -70,41 +70,45 @@ var listener_count = 0;
 function mkListener() {
   var receivedMessages = [];
   var listenSocket = dgram.createSocket('udp4');
-  listenSocket.addMembership(LOCAL_BROADCAST_HOST);
-
-  listenSocket.on('message', function(buf, rinfo) {
-    console.error('received %s from %j', util.inspect(buf.toString()), rinfo);
-    receivedMessages.push(buf);
-
-    if (receivedMessages.length == sendMessages.length) {
-      listenSocket.dropMembership(LOCAL_BROADCAST_HOST);
-      listenSocket.close();
-    }
-  });
-
-  listenSocket.on('close', function() {
-    console.error('listenSocket closed -- checking received messages');
-    var count = 0;
-    receivedMessages.forEach(function(buf) {
-      for (var i = 0; i < sendMessages.length; ++i) {
-        if (buf.toString() === sendMessages[i].toString()) {
-          count++;
-          break;
+  listenSocket.addMembership(LOCAL_BROADCAST_HOST, function(err) {
+    assert.ok(!err);
+
+    listenSocket.on('message', function(buf, rinfo) {
+      console.error('received %s from %j', util.inspect(buf.toString()), rinfo);
+      receivedMessages.push(buf);
+
+      if (receivedMessages.length == sendMessages.length) {
+        listenSocket.dropMembership(LOCAL_BROADCAST_HOST, function(err) {
+          assert.ok(!err);
+          listenSocket.close();
+        });
+      }
+    });
+
+    listenSocket.on('close', function() {
+      console.error('listenSocket closed -- checking received messages');
+      var count = 0;
+      receivedMessages.forEach(function(buf) {
+        for (var i = 0; i < sendMessages.length; ++i) {
+          if (buf.toString() === sendMessages[i].toString()) {
+            count++;
+            break;
+          }
         }
+      });
+      console.error('count %d', count);
+      //assert.strictEqual(count, sendMessages.length);
+    });
+
+    listenSocket.on('listening', function() {
+      listenSockets.push(listenSocket);
+      if (listenSockets.length == 3) {
+        sendSocket.sendNext();
       }
     });
-    console.error('count %d', count);
-    //assert.strictEqual(count, sendMessages.length);
+
+    listenSocket.bind(common.PORT);
   });
-
-  listenSocket.on('listening', function() {
-    listenSockets.push(listenSocket);
-    if (listenSockets.length == 3) {
-      sendSocket.sendNext();
-    }
-  });
-
-  listenSocket.bind(common.PORT);
 }
 
 mkListener();

diff --git a/test/simple/test-dgram-udp4.js b/test/simple/test-dgram-udp4.js
@@ -50,15 +50,18 @@ server.on('listening', function() {
     client.close();
     server.close();
   });
+  var sendReturned = false;
   client.send(message_to_send, 0, message_to_send.length,
-              server_port, 'localhost', function(err, bytes) {
+              server_port, '127.0.0.1', function(err, bytes) {
+        assert.ok(sendReturned);
         if (err) {
           console.log('Caught error in client send.');
           throw err;
         }
         console.log('client wrote ' + bytes + ' bytes.');
         assert.strictEqual(bytes, message_to_send.length);
       });
+  sendReturned = true;
   client.on('close',
             function() {
               if (server.fd === null) {