Skip to content

Commit

Permalink
[Fixes josdejong#58] Allocate Debug ports safely, so multiple pools d…
Browse files Browse the repository at this point in the history
…on’t accidentally overlap
  • Loading branch information
stefanpenner committed Mar 12, 2019
1 parent da0c602 commit 76e48fa
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 5 deletions.
10 changes: 6 additions & 4 deletions lib/Pool.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
var Promise = require('./Promise');
var WorkerHandler = require('./WorkerHandler');
var environment = require('./environment');

var DebugPortAllocator = require('./debug-port-allocator');
var DEBUG_PORT_ALLOCATOR = new DebugPortAllocator();
/**
* A pool to manage workers
* @param {String} [script] Optional worker script
Expand All @@ -24,7 +25,7 @@ function Pool(script, options) {

this.forkArgs = options.forkArgs || [];
this.forkOpts = options.forkOpts || {};
this.debugPortStart = options.debugPortStart || 43210;
this.debugPortStart = (options.debugPortStart || 43210);
this.nodeWorker = options.nodeWorker;

// configuration
Expand Down Expand Up @@ -243,7 +244,7 @@ Pool.prototype._getWorker = function() {
worker = new WorkerHandler(this.script, {
forkArgs: this.forkArgs,
forkOpts: this.forkOpts,
debugPort: this.debugPortStart + workers.length,
debugPort: DEBUG_PORT_ALLOCATOR.nextAvailableStartingAt(this.debugPortStart),
nodeWorker: this.nodeWorker
});
workers.push(worker);
Expand All @@ -260,6 +261,7 @@ Pool.prototype._getWorker = function() {
* @protected
*/
Pool.prototype._removeWorker = function(worker) {
DEBUG_PORT_ALLOCATOR.releasePort(worker.debugPort)
// terminate the worker (if not already terminated)
worker.terminate();
this._removeWorkerFromList(worker);
Expand Down Expand Up @@ -343,7 +345,7 @@ Pool.prototype._ensureMinWorkers = function() {
this.workers.push(new WorkerHandler(this.script, {
forkArgs: this.forkArgs,
forkOpts: this.forkOpts,
debugPort: this.debugPortStart + i
debugPort: DEBUG_PORT_ALLOCATOR.nextAvailableStartingAt(this.debugPortStart)
}));
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/WorkerHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ function objectToError (obj) {
function WorkerHandler(script, _options) {
this.script = script || getDefaultWorker();
var options = _options || {};
this.debugPort = options.debugPort;

if (environment.platform == 'browser') {
// check whether Worker is supported by the browser
Expand Down
28 changes: 28 additions & 0 deletions lib/debug-port-allocator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';

var MAX_PORTS = 65535;
module.exports = DebugPortAllocator;
function DebugPortAllocator() {
this.ports = Object.create(null);
this.length = 0;
}

DebugPortAllocator.prototype.nextAvailableStartingAt = function(starting) {
while (this.ports[starting] === true) {
starting++;
}

if (starting >= MAX_PORTS) {
throw new Error('WorkerPool debug port limit reached: ' + starting + '>= ' + MAX_PORTS );
}

this.ports[starting] = true;
this.length++;
return starting;
};

DebugPortAllocator.prototype.releasePort = function(port) {
delete this.ports[port];
this.length--;
};

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
"webpack": "^1.13.1"
},
"dependencies": {
"object-assign": "4.1.1"
"@babel/core": "^7.3.4",
"object-assign": "4.1.1",
"rsvp": "^4.8.4"
}
}
39 changes: 39 additions & 0 deletions test/debug-port-allocator-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

var assert = require('assert');
var DebugPortAllocator = require('../lib/debug-port-allocator');

describe('DebugPortAllocator', function () {
it('works', function() {
var allocator = new DebugPortAllocator();
assert.equal(allocator.length, 0);
assert.equal(allocator.nextAvailableStartingAt(5), 5);
assert.equal(allocator.length, 1);
assert.equal(allocator.nextAvailableStartingAt(5), 6);
assert.equal(allocator.length, 2);
assert.equal(allocator.nextAvailableStartingAt(5), 7);
assert.equal(allocator.length, 3);
assert.equal(allocator.nextAvailableStartingAt(4), 4);
assert.equal(allocator.length, 4);
assert.equal(allocator.nextAvailableStartingAt(4), 8);
assert.equal(allocator.length, 5);

allocator.releasePort(8);
assert.equal(allocator.length, 4);
assert.equal(allocator.nextAvailableStartingAt(8), 8);
allocator.releasePort(8);
allocator.releasePort(7);
allocator.releasePort(6);
allocator.releasePort(5);
allocator.releasePort(4);
assert.equal(allocator.length, 0);

assert.throws(function() {
allocator.nextAvailableStartingAt(65535);
}, /WorkerPool debug port limit reached/);
assert.throws(function() {
allocator.nextAvailableStartingAt(75535);
}, /WorkerPool debug port limit reached/);
})
});

0 comments on commit 76e48fa

Please sign in to comment.