Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Commit 485d5b5

Browse files
koichikkoichik
committed
dns: Force the DNS module to invoke callbacks asynchronously.
Fixes #1164.
1 parent e3413f0 commit 485d5b5

File tree

4 files changed

+152
-1
lines changed

4 files changed

+152
-1
lines changed

‎lib/dns_legacy.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,40 @@ var channel = new dns.Channel({SOCK_STATE_CB: function(socket, read, write) {
9191
updateTimer();
9292
}});
9393

94+
// c-ares invokes a callback either synchronously or asynchronously,
95+
// but the dns API should always invoke a callback asynchronously.
96+
//
97+
// This function makes sure that the callback is invoked asynchronously.
98+
// It returns a function that invokes the callback within nextTick().
99+
//
100+
// To avoid invoking unnecessary nextTick(), `immediately` property of
101+
// returned function should be set to true after c-ares returned.
102+
//
103+
// Usage:
104+
//
105+
// function someAPI(callback) {
106+
// callback = makeAsync(callback);
107+
// channel.someAPI(..., callback);
108+
// callback.immediately = true;
109+
// }
110+
function makeAsync(callback) {
111+
if (typeof callback !== 'function') {
112+
return callback;
113+
}
114+
return function asyncCallback() {
115+
if (asyncCallback.immediately) {
116+
// The API already returned, we can invoke the callback immediately.
117+
callback.apply(null, arguments);
118+
} else {
119+
var args = arguments;
120+
process.nextTick(function() {
121+
callback.apply(null, args);
122+
});
123+
}
124+
};
125+
}
126+
127+
94128
exports.resolve = function(domain, type_, callback_) {
95129
var type, callback;
96130
if (typeof(type_) == 'string') {
@@ -121,13 +155,17 @@ function familyToSym(family) {
121155

122156
exports.getHostByName = function(domain, family/*=4*/, callback) {
123157
if (typeof family === 'function') { callback = family; family = null; }
158+
callback = makeAsync(callback);
124159
channel.getHostByName(domain, familyToSym(family), callback);
160+
callback.immediately = true;
125161
};
126162

127163

128164
exports.getHostByAddr = function(address, family/*=4*/, callback) {
129165
if (typeof family === 'function') { callback = family; family = null; }
166+
callback = makeAsync(callback);
130167
channel.getHostByAddr(address, familyToSym(family), callback);
168+
callback.immediately = true;
131169
};
132170

133171

@@ -148,6 +186,7 @@ exports.lookup = function(domain, family, callback) {
148186
throw new Error('invalid argument: "family" must be 4 or 6');
149187
}
150188
}
189+
callback = makeAsync(callback);
151190

152191
if (!domain) {
153192
callback(null, null, family === 6 ? 6 : 4);
@@ -166,6 +205,7 @@ exports.lookup = function(domain, family, callback) {
166205
process.binding('net').getaddrinfo(domain, 4, function(err, domains4) {
167206
callback(err, domains4[0], 4);
168207
});
208+
callback.immediately = true;
169209
return;
170210
}
171211

@@ -183,6 +223,7 @@ exports.lookup = function(domain, family, callback) {
183223
callback(err, []);
184224
}
185225
});
226+
callback.immediately = true;
186227
return;
187228
}
188229

@@ -200,46 +241,63 @@ exports.lookup = function(domain, family, callback) {
200241
});
201242
}
202243
});
244+
callback.immediately = true;
203245
};
204246

205247

206248
exports.resolve4 = function(domain, callback) {
249+
callback = makeAsync(callback);
207250
channel.query(domain, dns.A, callback);
251+
callback.immediately = true;
208252
};
209253

210254

211255
exports.resolve6 = function(domain, callback) {
256+
callback = makeAsync(callback);
212257
channel.query(domain, dns.AAAA, callback);
258+
callback.immediately = true;
213259
};
214260

215261

216262
exports.resolveMx = function(domain, callback) {
263+
callback = makeAsync(callback);
217264
channel.query(domain, dns.MX, callback);
265+
callback.immediately = true;
218266
};
219267

220268

221269
exports.resolveTxt = function(domain, callback) {
270+
callback = makeAsync(callback);
222271
channel.query(domain, dns.TXT, callback);
272+
callback.immediately = true;
223273
};
224274

225275

226276
exports.resolveSrv = function(domain, callback) {
277+
callback = makeAsync(callback);
227278
channel.query(domain, dns.SRV, callback);
279+
callback.immediately = true;
228280
};
229281

230282

231283
exports.reverse = function(domain, callback) {
284+
callback = makeAsync(callback);
232285
channel.query(domain, dns.PTR, callback);
286+
callback.immediately = true;
233287
};
234288

235289

236290
exports.resolveNs = function(domain, callback) {
291+
callback = makeAsync(callback);
237292
channel.query(domain, dns.NS, callback);
293+
callback.immediately = true;
238294
};
239295

240296

241297
exports.resolveCname = function(domain, callback) {
298+
callback = makeAsync(callback);
242299
channel.query(domain, dns.CNAME, callback);
300+
callback.immediately = true;
243301
};
244302

245303
var resolveMap = { A: exports.resolve4,

‎lib/dns_uv.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,39 @@ function symToFamily(family) {
5252
}
5353
}
5454

55+
// c-ares invokes a callback either synchronously or asynchronously,
56+
// but the dns API should always invoke a callback asynchronously.
57+
//
58+
// This function makes sure that the callback is invoked asynchronously.
59+
// It returns a function that invokes the callback within nextTick().
60+
//
61+
// To avoid invoking unnecessary nextTick(), `immediately` property of
62+
// returned function should be set to true after c-ares returned.
63+
//
64+
// Usage:
65+
//
66+
// function someAPI(callback) {
67+
// callback = makeAsync(callback);
68+
// channel.someAPI(..., callback);
69+
// callback.immediately = true;
70+
// }
71+
function makeAsync(callback) {
72+
if (typeof callback !== 'function') {
73+
return callback;
74+
}
75+
return function asyncCallback() {
76+
if (asyncCallback.immediately) {
77+
// The API already returned, we can invoke the callback immediately.
78+
callback.apply(null, arguments);
79+
} else {
80+
var args = arguments;
81+
process.nextTick(function() {
82+
callback.apply(null, args);
83+
});
84+
}
85+
};
86+
}
87+
5588

5689
// Easy DNS A/AAAA look up
5790
// lookup(domain, [family,] callback)
@@ -68,6 +101,7 @@ exports.lookup = function(domain, family, callback) {
68101
throw new Error('invalid argument: `family` must be 4 or 6');
69102
}
70103
}
104+
callback = makeAsync(callback);
71105

72106
if (!domain) {
73107
callback(null, null, family === 6 ? 6 : 4);
@@ -87,6 +121,7 @@ exports.lookup = function(domain, family, callback) {
87121
process.binding('net').getaddrinfo(domain, 4, function(err, domains4) {
88122
callback(err, domains4[0], 4);
89123
});
124+
callback.immediately = true;
90125
return {};
91126
} */
92127

@@ -103,6 +138,7 @@ exports.lookup = function(domain, family, callback) {
103138
throw errnoException(errno, 'getHostByName');
104139
}
105140

141+
callback.immediately = true;
106142
return wrap;
107143
};
108144

@@ -119,11 +155,13 @@ function resolver(bindingName) {
119155
}
120156
}
121157

158+
callback = makeAsync(callback);
122159
var wrap = binding(name, onanswer);
123160
if (!wrap) {
124161
throw errnoException(errno, bindingName);
125162
}
126163

164+
callback.immediately = true;
127165
return wrap;
128166
}
129167
}

‎test/disabled/test-dns.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,12 @@ while (i--) {
6060
}
6161

6262
// CNAME should resolve
63+
var resolveCNAME = 'before';
6364
dns.resolve('labs.nrcmedia.nl', 'CNAME', function(err, result) {
6465
assert.deepEqual(result, ['nrcmedia.nl']);
66+
assert.equal(resolveCNAME, 'beforeafter');
6567
});
68+
resolveCNAME += 'after';
6669

6770
// CNAME should not resolve
6871
dns.resolve('nrcmedia.nl', 'CNAME', function(err, result) {
@@ -74,6 +77,7 @@ function checkDnsRecord(host, record) {
7477
myRecord = record;
7578
return function(err, stdout) {
7679
var expected = [];
80+
var footprints = 'before';
7781
if (stdout.length)
7882
expected = stdout.substr(0, stdout.length - 1).split('\n');
7983

@@ -94,6 +98,7 @@ function checkDnsRecord(host, record) {
9498

9599
child_process.exec(reverseCmd, checkReverse(ip));
96100
}
101+
assert.equal(footprints, 'beforeafter');
97102
});
98103
break;
99104
case 'MX':
@@ -106,6 +111,7 @@ function checkDnsRecord(host, record) {
106111
strResult.push(result[ll].priority + ' ' + result[ll].exchange);
107112
}
108113
cmpResults(expected, strResult, ttl, cname);
114+
assert.equal(footprints, 'beforeafter');
109115
});
110116
break;
111117
case 'TXT':
@@ -118,6 +124,7 @@ function checkDnsRecord(host, record) {
118124
strResult.push('"' + result[ll] + '"');
119125
}
120126
cmpResults(expected, strResult, ttl, cname);
127+
assert.equal(footprints, 'beforeafter');
121128
});
122129
break;
123130
case 'SRV':
@@ -133,9 +140,11 @@ function checkDnsRecord(host, record) {
133140
result[ll].name);
134141
}
135142
cmpResults(expected, strResult, ttl, cname);
143+
assert.equal(footprints, 'beforeafter');
136144
});
137145
break;
138146
}
147+
footprints += 'after';
139148
}
140149
}
141150

@@ -174,3 +183,46 @@ function cmpResults(expected, result, ttl, cname) {
174183
' was equal to expected ' + expected[ll]);
175184
}
176185
}
186+
187+
// #1164
188+
var getHostByName = 'before';
189+
dns.getHostByName('localhost', function() {
190+
assert.equal(getHostByName, 'beforeafter');
191+
});
192+
getHostByName += 'after';
193+
194+
var getHostByAddr = 'before';
195+
dns.getHostByAddr('127.0.0.1', function() {
196+
assert.equal(getHostByAddr, 'beforeafter');
197+
});
198+
getHostByAddr += 'after';
199+
200+
var lookupEmpty = 'before';
201+
dns.lookup('', function() {
202+
assert.equal(lookupEmpty, 'beforeafter');
203+
});
204+
lookupEmpty += 'after';
205+
206+
var lookupIp = 'before';
207+
dns.lookup('127.0.0.1', function() {
208+
assert.equal(lookupIp, 'beforeafter');
209+
});
210+
lookupIp += 'after';
211+
212+
var lookupIp4 = 'before';
213+
dns.lookup('127.0.0.1', 4, function() {
214+
assert.equal(lookupIp4, 'beforeafter');
215+
});
216+
lookupIp4 += 'after';
217+
218+
var lookupIp6 = 'before';
219+
dns.lookup('ietf.org', 6, function() {
220+
assert.equal(lookupIp6, 'beforeafter');
221+
});
222+
lookupIp6 += 'after';
223+
224+
var lookupLocal = 'before';
225+
dns.lookup('localhost', function() {
226+
assert.equal(lookupLocal, 'beforeafter');
227+
});
228+
lookupLocal += 'after';

‎test/simple/test-dgram-multicast.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,10 @@ function mkListener() {
7878

7979
if (receivedMessages.length == sendMessages.length) {
8080
listenSocket.dropMembership(LOCAL_BROADCAST_HOST);
81-
listenSocket.close();
81+
process.nextTick(function() { // TODO should be changed to below.
82+
// listenSocket.dropMembership(LOCAL_BROADCAST_HOST, function() {
83+
listenSocket.close();
84+
});
8285
}
8386
});
8487

0 commit comments

Comments
 (0)