Browse Source

Pool getPort race condition

It appears that when requesting multiple ports asynchronously,
that a port is given out multiple times, which crashes single
Tor processes. Such a crash brings down the whole system as of
now, which might or might not be desired.
This is caused by a race condition on the port numbers. This
commit fixes the issue if a number of Tor instances for the pool
is requested. If the pool is populated with an array of definitions,
the user has to make sure to include the ports in the definition,
otherwise the race condition still persists. If this should still be
allowed is up for discussion or if a check should be included in the
TorPool create function.
Roman Brunner 5 years ago
parent
commit
f89e4b20c5
5 changed files with 310 additions and 332 deletions
  1. 3 2
      .gitignore
  2. 282 325
      package-lock.json
  3. 2 0
      package.json
  4. 15 0
      src/TorPool.js
  5. 8 5
      src/TorProcess.js

+ 3 - 2
.gitignore

@@ -1,5 +1,6 @@
 node_modules
 node_modules
-npm-debug.log
+*.log
 .env
 .env
 .vscode
 .vscode
-.DS_Store
+.DS_Store
+

File diff suppressed because it is too large
+ 282 - 325
package-lock.json


+ 2 - 0
package.json

@@ -31,6 +31,8 @@
     "socks-proxy-agent": "^4.0.1"
     "socks-proxy-agent": "^4.0.1"
   },
   },
   "dependencies": {
   "dependencies": {
+    "arrify": "^2.0.1",
+    "async-mutex": "^0.1.4",
     "bluebird": "^3.5.2",
     "bluebird": "^3.5.2",
     "del": "^3.0.0",
     "del": "^3.0.0",
     "eventemitter3": "^3.1.0",
     "eventemitter3": "^3.1.0",

+ 15 - 0
src/TorPool.js

@@ -27,7 +27,9 @@ const Promise = require("bluebird");
 const _ = require('lodash');
 const _ = require('lodash');
 const WeightedList = require('js-weighted-list');
 const WeightedList = require('js-weighted-list');
 
 
+const getPort = require('get-port');
 const TorProcess = require('./TorProcess');
 const TorProcess = require('./TorProcess');
+const { Mutex } = require('async-mutex');
 
 
 Promise.promisifyAll(fs);
 Promise.promisifyAll(fs);
 
 
@@ -462,6 +464,19 @@ class TorPool extends EventEmitter {
 		
 		
 		if (typeof(instances) === 'number') {
 		if (typeof(instances) === 'number') {
 			instances = Array.from(Array(instances)).map(() => ({}));
 			instances = Array.from(Array(instances)).map(() => ({}));
+			const lock = new Mutex();
+			instances = await Promise.all(instances.map(async(instance) => {
+				instance.ports = {};
+				let release = await lock.acquire();
+				try {
+					instance.ports.dns_port = await getPort();
+					instance.ports.socks_port = await getPort();
+					instance.ports.control_port = await getPort();
+				} finally {
+					release();
+				}
+				return instance;
+			}));
 		}
 		}
 		return await this.add(instances);
 		return await this.add(instances);
 	}
 	}

+ 8 - 5
src/TorProcess.js

@@ -52,6 +52,7 @@ class TorProcess extends EventEmitter {
 		definition.Config = definition.Config || {};
 		definition.Config = definition.Config || {};
 
 
 		this._definition = definition;
 		this._definition = definition;
+		this._ports = definition.ports || {};
 
 
 		/**
 		/**
 		 * Path to the Tor executable.
 		 * Path to the Tor executable.
@@ -263,10 +264,12 @@ class TorProcess extends EventEmitter {
 	 * @returns {Promise<ChildProcess>} - The process that has been created.
 	 * @returns {Promise<ChildProcess>} - The process that has been created.
 	 */
 	 */
 	async create() {
 	async create() {
-		this._ports = {};
-		let dnsPort = this._ports.dns_port = await getPort();
-		let socksPort = this._ports.socks_port =  await getPort();
-		let controlPort = this._ports.control_port = await getPort();
+		let dnsPort = this._ports.dns_port || await getPort();
+		let socksPort = this._ports.socks_port || await getPort();
+		let controlPort = this._ports.control_port || await getPort();
+		this.logger.info(`[tor-${this.instance_name}]: DNS PORT = ${dnsPort}`);
+		this.logger.info(`[tor-${this.instance_name}]: SOCKS PORT = ${socksPort}`);
+		this.logger.info(`[tor-${this.instance_name}]: CONTROL PORT = ${controlPort}`);
 		Object.freeze(this._ports);
 		Object.freeze(this._ports);
 
 
 		let options = {
 		let options = {
@@ -403,7 +406,7 @@ class TorProcess extends EventEmitter {
 				 * @returns {Error}
 				 * @returns {Error}
 				 */
 				 */
 				this.emit('error', new Error(msg));
 				this.emit('error', new Error(msg));
-				this.logger.error(`[tor-${this.instance_name}]: ${msg}`);
+				this.logger.error(`[tor-${this.instance_name}]: ${text}`);
 			}
 			}
 
 
 			else if (text.indexOf('[notice]') !== -1) {
 			else if (text.indexOf('[notice]') !== -1) {

Some files were not shown because too many files changed in this diff