From 8197292cdd252a57c879b83355ef62845c7c0a90 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Mon, 5 May 2014 22:31:05 -0500 Subject: [PATCH 1/3] Add failing test for listening socket domains --- test/protect.js | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/protect.js b/test/protect.js index 5e062916e..f22e5707b 100755 --- a/test/protect.js +++ b/test/protect.js @@ -4,6 +4,7 @@ var Events = require('events'); var Lab = require('lab'); var Hapi = require('..'); var Hoek = require('hoek'); +var Nipple = require('nipple'); // Declare internals @@ -43,6 +44,48 @@ describe('Protect', function () { }); }); + + it('does not propagate the domain outside of request lifetime', function (done) { + + // The server must be run without the test domain to demonstrate the issue. + // In isolation this is unnecessary but multiple tests somehow leave the domain + // state in a somehow broken state. + while (process.domain) { + process.domain.exit(); + } + + var server = new Hapi.Server(0); + var executed = false; + + var handler = function (request, reply) { + + expect(process.domain).to.equal(request.domain); + expect(request.raw.req.socket.domain).to.equal(null); + + if (!executed) { + executed = true; + Nipple.request('GET', 'http://localhost:' + server.info.port + '/', {}, function (err, res) { + expect(res.statusCode).to.equal(200); + reply('ok'); + }); + } else { + reply('ok'); + } + }; + + server.route({ method: 'GET', path: '/', handler: handler }); + server.start(function() { + + require('domain').log = true; + Nipple.request('GET', 'http://localhost:' + server.info.port + '/', {}, function (err, res) { + + server.stop(); + expect(res.statusCode).to.equal(200); + done(); + }); + }); + }); + it('catches error when handler throws twice after reply() is called', function (done) { var server = new Hapi.Server({ debug: false }); From e3d67835a7bcde46675da9b3cc6eb09d155be458 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Mon, 5 May 2014 22:42:58 -0500 Subject: [PATCH 2/3] Avoid async operations while protect is running Sockets were being associated with random unrelated requests that were executing when the incoming socket connection is made due to the domain entry and exit spanning multiple event loop cycles. Additionally this pattern was prone for other concurrent execution issues where the domain stack is left in an unexpected state due to races between two concurrent requests, i.e. ``` request1.enter(); request2.enter(); request1.exit(); ``` Causes the domains stack to be cleared of both request1 and request2 even though request2 is still executing. --- lib/protect.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/protect.js b/lib/protect.js index e035b8119..c82c9a29c 100755 --- a/lib/protect.js +++ b/lib/protect.js @@ -17,9 +17,6 @@ exports = module.exports = internals.Protect = function (parent) { this._parent = parent; this._error = null; - - this._baseDomain = Domain.createDomain(); // Envelope domain to force exit all nested inner domains - this.domain = Domain.createDomain(); this.domain.on('error', function (err) { @@ -42,7 +39,6 @@ internals.Protect.prototype.run = function (next, setup) { // setup: fu var finish = function (/* arguments */) { - self._baseDomain.exit(); // Forces exit from this.domain along with any nested domains self._error = null; return next.apply(null, arguments); @@ -57,9 +53,7 @@ internals.Protect.prototype.run = function (next, setup) { // setup: fu var protect = function (run) { - self._baseDomain.enter(); - self.domain.enter(); - run(); + self.domain.run(run); }; setup(protect, finish); From 18f7c5fc3d09961465601428b5abd41fa7f64429 Mon Sep 17 00:00:00 2001 From: kpdecker Date: Mon, 12 May 2014 12:04:59 -0500 Subject: [PATCH 3/3] Dump out of the request domain before sending --- lib/protect.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/protect.js b/lib/protect.js index c82c9a29c..6cde93712 100755 --- a/lib/protect.js +++ b/lib/protect.js @@ -48,6 +48,9 @@ internals.Protect.prototype.run = function (next, setup) { // setup: fu this._error = function (err) { + // Exit out of the implict request domain so we can isolate user and framework code. + self.domain.exit(); + return finish(Boom.badImplementation('Uncaught error', err)); };