Fix in-process race condition (#36)

index.js

@@ -1,6 +1,25 @@
 'use strict';
 const net = require('net');
 
+class Locked extends Error {
+	constructor(port) {
+		super(`${port} is locked`);
+	}
+}
+
+const lockedPorts = {
+	old: new Set(),
+	young: new Set()
+};
+
+// On this interval the old locked ports are discarded
+// the young locked ports are moved to old locked ports
+// and a new young set for locked ports is created
+const releaseOldLockedPortsIntervalMs = 1000 * 15;
+
+// Lazily create interval on first use
+let interval = null;
+
 const getAvailablePort = options => new Promise((resolve, reject) => {
 	const server = net.createServer();
 	server.unref();
@@ -28,11 +47,30 @@ module.exports = async options => {
 		ports = typeof options.port === 'number' ? [options.port] : options.port;
 	}
 
+	if (interval === null) {
+		interval = setInterval(() => {
+			lockedPorts.old = lockedPorts.young;
+			lockedPorts.young = new Set();
+		}, releaseOldLockedPortsIntervalMs);
+
+		interval.unref();
+	}
+
 	for (const port of portCheckSequence(ports)) {
 		try {
-			return await getAvailablePort({...options, port}); // eslint-disable-line no-await-in-loop
+			let availablePort = await getAvailablePort({...options, port}); // eslint-disable-line no-await-in-loop
+			while (lockedPorts.old.has(availablePort) || lockedPorts.young.has(availablePort)) {
+				if (port !== 0) {
+					throw new Locked(port);
+				}
+
+				availablePort = await getAvailablePort({...options, port}); // eslint-disable-line no-await-in-loop
+			}
+
+			lockedPorts.young.add(availablePort);
+			return availablePort;
 		} catch (error) {
-			if (error.code !== 'EADDRINUSE') {
+			if (error.code !== 'EADDRINUSE' && !(error instanceof Locked)) {
 				throw error;
 			}
 		}

readme.md

@@ -91,7 +91,9 @@ Last port of the range. Must be in the range `1024`...`65535` and must be greate
 
 ## Beware
 
-There is a very tiny chance of a race condition if another service starts using the same port number as you in between the time you get the port number and you actually start using it.
+There is a very tiny chance of a race condition if another process starts using the same port number as you in between the time you get the port number and you actually start using it.
+
+Race conditions in the same process are mitigated against by using a lightweight locking mechanism where a port will be held for a minimum of 15 seconds and a maximum of 30 seconds before being released again.
 
 
 ## Related

test.js

@@ -44,7 +44,7 @@ test('port can be bound to IPv4 host when promise resolves', async t => {
 });
 
 test('preferred port given IPv4 host', async t => {
-	const desiredPort = 8080;
+	const desiredPort = 8081;
 	const port = await getPort({
 		port: desiredPort,
 		host: '0.0.0.0'
@@ -127,3 +127,20 @@ test('makeRange produces valid ranges', t => {
 	t.deepEqual([...getPort.makeRange(1024, 1025)], [1024, 1025]);
 	t.deepEqual([...getPort.makeRange(1024, 1027)], [1024, 1025, 1026, 1027]);
 });
+
+test('ports are locked for up to 30 seconds', async t => {
+	// Speed up the test by overriding `setInterval`.
+	const {setInterval} = global;
+	global.setInterval = (fn, timeout) => setInterval(fn, timeout / 100);
+
+	delete require.cache[require.resolve('.')];
+	const getPort = require('.');
+	const timeout = promisify(setTimeout);
+	const port = await getPort();
+	const port2 = await getPort({port});
+	t.not(port2, port);
+	await timeout(300); // 30000 / 100
+	const port3 = await getPort({port});
+	t.is(port3, port);
+	global.setInterval = setInterval;
+});