From 85bd4220cda2b99a8e43602c937df53b53b87128 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 20 Jan 2016 22:55:00 -0800 Subject: [PATCH 1/4] test: remove race condition in http flood test Timer race results in some flakiness on slower devices in CI. --- test/parallel/test-http-pipeline-flood.js | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/test/parallel/test-http-pipeline-flood.js b/test/parallel/test-http-pipeline-flood.js index 571636ffed2e5f..f6b70216839431 100644 --- a/test/parallel/test-http-pipeline-flood.js +++ b/test/parallel/test-http-pipeline-flood.js @@ -24,8 +24,6 @@ switch (process.argv[2]) { function parent() { const http = require('http'); const bigResponse = new Buffer(10240).fill('x'); - var gotTimeout = false; - var childClosed = false; var requests = 0; var connections = 0; var backloggedReqs = 0; @@ -57,20 +55,16 @@ function parent() { const spawn = require('child_process').spawn; const args = [__filename, 'child']; const child = spawn(process.execPath, args, { stdio: 'inherit' }); - child.on('close', function() { - childClosed = true; + child.on('close', common.mustCall(function() { server.close(); - }); + })); - server.setTimeout(common.platformTimeout(200), function(conn) { - gotTimeout = true; + server.setTimeout(common.platformTimeout(200), common.mustCall(function() { child.kill(); - }); + })); }); process.on('exit', function() { - assert(gotTimeout); - assert(childClosed); assert.equal(connections, 1); }); } @@ -85,11 +79,7 @@ function child() { req = new Array(10241).join(req); - conn.on('connect', function() { - // Terminate child after flooding. - setTimeout(function() { conn.destroy(); }, common.platformTimeout(1000)); - write(); - }); + conn.on('connect', write); conn.on('drain', write); From a94ea4c44f216bf56daa7e545c37f31c3030607e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 23 Jan 2016 22:46:27 -0800 Subject: [PATCH 2/4] fixup --- test/parallel/test-http-pipeline-flood.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http-pipeline-flood.js b/test/parallel/test-http-pipeline-flood.js index f6b70216839431..3ed94e4476ad50 100644 --- a/test/parallel/test-http-pipeline-flood.js +++ b/test/parallel/test-http-pipeline-flood.js @@ -12,6 +12,9 @@ const assert = require('assert'); // Normally when the writable stream emits a 'drain' event, the server then // uncorks the readable stream, although we arent testing that part here. +// The issue being tested exists in Node.js 0.10.20 and is resolved in 0.10.21 +// and newer. + switch (process.argv[2]) { case undefined: return parent(); @@ -59,7 +62,7 @@ function parent() { server.close(); })); - server.setTimeout(common.platformTimeout(200), common.mustCall(function() { + server.setTimeout(common.platformTimeout(10), common.mustCall(function() { child.kill(); })); }); @@ -79,7 +82,11 @@ function child() { req = new Array(10241).join(req); - conn.on('connect', write); + conn.on('connect', function() { + // Terminate child after flooding. + setTimeout(function() { conn.destroy(); }, 500); + write(); + }); conn.on('drain', write); From fd3ca84f92da0ff010038efffecfd1fb039720b9 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 23 Jan 2016 23:13:50 -0800 Subject: [PATCH 3/4] fixup --- test/parallel/test-http-pipeline-flood.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/parallel/test-http-pipeline-flood.js b/test/parallel/test-http-pipeline-flood.js index 3ed94e4476ad50..4b52d8db359e9d 100644 --- a/test/parallel/test-http-pipeline-flood.js +++ b/test/parallel/test-http-pipeline-flood.js @@ -62,7 +62,7 @@ function parent() { server.close(); })); - server.setTimeout(common.platformTimeout(10), common.mustCall(function() { + server.setTimeout(10, common.mustCall(function() { child.kill(); })); }); @@ -82,13 +82,10 @@ function child() { req = new Array(10241).join(req); - conn.on('connect', function() { - // Terminate child after flooding. - setTimeout(function() { conn.destroy(); }, 500); - write(); - }); + conn.on('connect', write); - conn.on('drain', write); + // `drain` should fire once and only once + conn.on('drain', common.mustCall(write)); function write() { while (false !== conn.write(req, 'ascii')); From ef5ec877c794e6aaf577f70d711b9ad4d0a80376 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 23 Jan 2016 23:38:52 -0800 Subject: [PATCH 4/4] fixup: increase socket timeout --- test/parallel/test-http-pipeline-flood.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-http-pipeline-flood.js b/test/parallel/test-http-pipeline-flood.js index 4b52d8db359e9d..d291ccdb1776b5 100644 --- a/test/parallel/test-http-pipeline-flood.js +++ b/test/parallel/test-http-pipeline-flood.js @@ -62,7 +62,7 @@ function parent() { server.close(); })); - server.setTimeout(10, common.mustCall(function() { + server.setTimeout(200, common.mustCall(function() { child.kill(); })); });