Simplify extension duplication prevention station

Since we will only have builtin extensions for a while, the
more-complicated, watchdog-based extension duplication prevention has
been replaced with one that treats the extension URL as equivalent to
the extension ID. This is true only for built-in extensions, so once we
start supporting third-party extensions we will likely need to bring
back the watchdog (commit e4381b4693) and
the more-complex duplication prevention (commit
670e51d335).

The code here should be treated as a last resort: ideally the GUI should
check if a particular extension is loaded (maybe by calling the
Extension Manager's new `isExtensionLoaded` method) and avoid calling
`loadExtensionURL` if it is.
This commit is contained in:
Christopher Willis-Ford 2017-10-31 12:35:18 -07:00
parent 670e51d335
commit ae878b2050
3 changed files with 18 additions and 122 deletions

View file

@ -38,7 +38,7 @@ class CentralDispatch extends SharedDispatch {
/** /**
* Set a local object as the global provider of the specified service. * Set a local object as the global provider of the specified service.
* WARNING: Any method on the provider can be called from any worker within the dispatch system. * WARNING: Any method on the provider can be called from any worker within the dispatch system.
* @param {string} service - a globally unique string identifying this service. Examples: 'vm', 'gui'. * @param {string} service - a globally unique string identifying this service. Examples: 'vm', 'gui', 'extension9'.
* @param {object} provider - a local object which provides this service. * @param {object} provider - a local object which provides this service.
* @returns {Promise} - a promise which will resolve once the service is registered. * @returns {Promise} - a promise which will resolve once the service is registered.
*/ */
@ -55,19 +55,6 @@ class CentralDispatch extends SharedDispatch {
} }
} }
/**
* Unregister a service without regard to which object or worker might provide it.
* @param {string} service - the globally unique string provided to `setService` when the service was registered.
* @see {setService}
*/
clearService (service) {
if (this.services.hasOwnProperty(service)) {
delete this.services[service];
} else {
log.warn(`Central dispatch can't clear unknown service ${service}`);
}
}
/** /**
* Add a worker to the message dispatch system. The worker must implement a compatible message dispatch framework. * Add a worker to the message dispatch system. The worker must implement a compatible message dispatch framework.
* The dispatcher will immediately attempt to "handshake" with the worker. * The dispatcher will immediately attempt to "handshake" with the worker.
@ -85,25 +72,6 @@ class CentralDispatch extends SharedDispatch {
} }
} }
/**
* Remove a service provider from the central dispatch service, un-registering any services it provides.
* If the provider is a worker its dispatch service will be shut down.
* The worker itself will NOT be terminated by this call.
* @param {Worker|object} provider - the worker or object instance to be removed.
*/
removeProvider (provider) {
const workerIndex = this.workers.indexOf(provider);
if (workerIndex !== -1) {
/** @TODO Should we ask the worker to shut down? If so, owner must wait before terminating the worker. */
this.workers.splice(workerIndex, 1);
}
for (const serviceName in Object.keys(this.services)) {
if (this.services[serviceName] === provider) {
delete this.services[serviceName];
}
}
}
/** /**
* Fetch the service provider object for a particular service name. * Fetch the service provider object for a particular service name.
* @override * @override

View file

@ -39,24 +39,6 @@ class SharedDispatch {
* @type {int} * @type {int}
*/ */
this.nextResponseId = 0; this.nextResponseId = 0;
/**
* While processing a call in _onMessage, this will be set to the originating object.
* At all other times it will be null.
* @type {object}
* @private
*/
this._callingWorker = null;
}
/**
* Central dispatch: while processing a call from a worker, this will be set to the worker originating that call.
* Worker dispatch: while processing a call from the main thread, this will be set to the global object.
* At all other times it will be null.
* @returns {object} - the worker originating the current call, if any.
*/
get callingWorker () {
return this._callingWorker;
} }
/** /**
@ -202,15 +184,10 @@ class SharedDispatch {
message.args = message.args || []; message.args = message.args || [];
let promise; let promise;
if (message.service) { if (message.service) {
try { if (message.service === 'dispatch') {
this._callingWorker = worker; promise = this._onDispatchMessage(worker, message);
if (message.service === 'dispatch') { } else {
promise = this._onDispatchMessage(worker, message); promise = this.call(message.service, message.method, ...message.args);
} else {
promise = this.call(message.service, message.method, ...message.args);
}
} finally {
this._callingWorker = null;
} }
} else if (typeof message.responseId === 'undefined') { } else if (typeof message.responseId === 'undefined') {
log.error(`Dispatch caught malformed message from a worker: ${JSON.stringify(event)}`); log.error(`Dispatch caught malformed message from a worker: ${JSON.stringify(event)}`);

View file

@ -71,20 +71,12 @@ class ExtensionManager {
this.pendingWorkers = []; this.pendingWorkers = [];
/** /**
* Set of loaded extension IDs. For built-in extensions the "URL" is the same as the ID; they differ in general. * Set of loaded extension URLs/IDs (equivalent for built-in extensions).
* @type {Set.<string>} * @type {Set.<string>}
* @private * @private
*/ */
this._loadedExtensions = new Set(); this._loadedExtensions = new Set();
/**
* Set of workers currently being monitored by `_startWorkerWatchdog`.
* @see {_startWorkerWatchdog}
* @type {Set.<object>}
* @private
*/
this._activeWatchdogs = new Set();
/** /**
* Keep a reference to the runtime so we can construct internal extension objects. * Keep a reference to the runtime so we can construct internal extension objects.
* TODO: remove this in favor of extensions accessing the runtime as a service. * TODO: remove this in favor of extensions accessing the runtime as a service.
@ -101,7 +93,7 @@ class ExtensionManager {
* Check whether an extension is registered or is in the process of loading. This is intended to control loading or * Check whether an extension is registered or is in the process of loading. This is intended to control loading or
* adding extensions so it may return `true` before the extension is ready to be used. Use the promise returned by * adding extensions so it may return `true` before the extension is ready to be used. Use the promise returned by
* `loadExtensionURL` if you need to wait until the extension is truly ready. * `loadExtensionURL` if you need to wait until the extension is truly ready.
* @param {string} extensionID - the ID (not URL) of the extension. * @param {string} extensionID - the ID of the extension.
* @returns {boolean} - true if loaded, false otherwise. * @returns {boolean} - true if loaded, false otherwise.
*/ */
isExtensionLoaded (extensionID) { isExtensionLoaded (extensionID) {
@ -115,6 +107,13 @@ class ExtensionManager {
*/ */
loadExtensionURL (extensionURL) { loadExtensionURL (extensionURL) {
if (builtinExtensions.hasOwnProperty(extensionURL)) { if (builtinExtensions.hasOwnProperty(extensionURL)) {
/** @TODO dupe handling for non-builtin extensions. See commit 670e51d33580e8a2e852b3b038bb3afc282f81b9 */
if (this.isExtensionLoaded(extensionURL)) {
const message = `Rejecting attempt to load a second extension with ID ${extensionURL}`;
log.warn(message);
return Promise.reject(new Error(message));
}
const extension = builtinExtensions[extensionURL]; const extension = builtinExtensions[extensionURL];
const extensionInstance = new extension(this.runtime); const extensionInstance = new extension(this.runtime);
return this._registerInternalExtension(extensionInstance); return this._registerInternalExtension(extensionInstance);
@ -125,10 +124,7 @@ class ExtensionManager {
const ExtensionWorker = require('worker-loader?name=extension-worker.js!./extension-worker'); const ExtensionWorker = require('worker-loader?name=extension-worker.js!./extension-worker');
this.pendingExtensions.push({extensionURL, resolve, reject}); this.pendingExtensions.push({extensionURL, resolve, reject});
dispatch.addWorker(new ExtensionWorker());
const worker = new ExtensionWorker();
dispatch.addWorker(worker);
this._startWorkerWatchdog(worker);
}); });
} }
@ -158,9 +154,6 @@ class ExtensionManager {
const workerInfo = this.pendingWorkers[id]; const workerInfo = this.pendingWorkers[id];
delete this.pendingWorkers[id]; delete this.pendingWorkers[id];
if (e) { if (e) {
log.warn(`Extension manager forcibly terminating worker for extension with ID ${id}, URL ${
workerInfo.extensionURL} due to error during initialization`);
workerInfo.worker.terminate();
workerInfo.reject(e); workerInfo.reject(e);
} else { } else {
workerInfo.resolve(id); workerInfo.resolve(id);
@ -188,22 +181,9 @@ class ExtensionManager {
*/ */
_registerExtensionInfo (serviceName, extensionInfo) { _registerExtensionInfo (serviceName, extensionInfo) {
extensionInfo = this._prepareExtensionInfo(serviceName, extensionInfo); extensionInfo = this._prepareExtensionInfo(serviceName, extensionInfo);
if (this.isExtensionLoaded(extensionInfo.id)) { dispatch.call('runtime', '_registerExtensionPrimitives', extensionInfo).catch(e => {
const message = `Ignoring attempt to load a second extension with ID ${extensionInfo.id}`; log.error(`Failed to register primitives for extension on service ${serviceName}: ${JSON.stringify(e)}`);
log.warn(message); });
dispatch.clearService(serviceName);
} else {
dispatch.call('runtime', '_registerExtensionPrimitives', extensionInfo).then(
() => {
this._loadedExtensions.add(extensionInfo.id);
if (dispatch.callingWorker) {
this._stopWorkerWatchdog(dispatch.callingWorker);
}
},
e => {
log.error(`Failed to register primitives for extension "${extensionInfo.id}": ${e.message}`);
});
}
} }
/** /**
@ -272,35 +252,6 @@ class ExtensionManager {
} }
return blockInfo; return blockInfo;
} }
/**
* Start a watchdog to terminate this worker unless it registers an extension before the timeout.
* @param {object} worker - the worker to watch.
* @private
*/
_startWorkerWatchdog (worker) {
const timeout = 5 * 1000; // 5 seconds
this._activeWatchdogs.add(worker);
setTimeout(() => {
if (this._activeWatchdogs.has(worker)) {
this._activeWatchdogs.delete(worker);
log.warn(`Worker watchdog timed out. Terminating worker.`);
dispatch.removeProvider(worker);
worker.terminate();
}
}, timeout);
}
/**
* Mark this worker as "safe" from the watchdog.
* @param {object} worker - the worker to mark as safe.
* @see {_startWorkerWatchdog}
* @private
*/
_stopWorkerWatchdog (worker) {
this._activeWatchdogs.delete(worker);
}
} }
module.exports = ExtensionManager; module.exports = ExtensionManager;