From 3b390b74ff032f62f2d95777f78a8fbee79e3caf Mon Sep 17 00:00:00 2001 From: Andrey Goncharov Date: Mon, 28 Feb 2022 03:50:34 -0800 Subject: [PATCH] Track client connections and autostop server add-ons when all clients leave Reviewed By: mweststrate Differential Revision: D34045584 fbshipit-source-id: 1ad0cfffb9d304f0359c973d76d6956f7e932f72 --- desktop/flipper-common/src/server-types.tsx | 10 ++++- .../src/plugin/DevicePlugin.tsx | 18 ++++++-- desktop/flipper-plugin/src/plugin/Plugin.tsx | 18 ++++++-- .../src/FlipperServerImpl.tsx | 9 ++-- .../src/comms/ServerController.tsx | 1 + .../src/devices/ServerDevice.tsx | 1 + .../src/plugins/PluginManager.tsx | 25 ++++++----- .../src/plugins/ServerAddOn.tsx | 43 +++++++++++-------- .../src/utils/createServerAddOnControls.tsx | 8 ++-- 9 files changed, 87 insertions(+), 46 deletions(-) diff --git a/desktop/flipper-common/src/server-types.tsx b/desktop/flipper-common/src/server-types.tsx index c853114e7..02e6dca5f 100644 --- a/desktop/flipper-common/src/server-types.tsx +++ b/desktop/flipper-common/src/server-types.tsx @@ -249,8 +249,14 @@ export type FlipperServerCommands = { path: string, ) => Promise; 'plugins-remove-plugins': (names: string[]) => Promise; - 'plugins-server-add-on-start': (pluginName: string) => Promise; - 'plugins-server-add-on-stop': (pluginName: string) => Promise; + 'plugins-server-add-on-start': ( + pluginName: string, + owner: string, + ) => Promise; + 'plugins-server-add-on-stop': ( + pluginName: string, + owner: string, + ) => Promise; 'doctor-get-healthchecks': ( settings: FlipperDoctor.HealthcheckSettings, ) => Promise; diff --git a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx index 7064556d7..481f7b1f8 100644 --- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx +++ b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx @@ -101,8 +101,13 @@ export class SandyDevicePluginInstance extends BasePluginInstance { private startServerAddOn() { const {serverAddOn, name} = this.definition.details; if (serverAddOn) { - this.serverAddOnControls.start(name).catch((e) => { - console.warn('Failed to start a server add on', name, e); + this.serverAddOnControls.start(name, this.device.serial).catch((e) => { + console.warn( + 'Failed to start a server add on', + name, + this.device.serial, + e, + ); }); } } @@ -110,8 +115,13 @@ export class SandyDevicePluginInstance extends BasePluginInstance { private stopServerAddOn() { const {serverAddOn, name} = this.definition.details; if (serverAddOn) { - this.serverAddOnControls.stop(name).catch((e) => { - console.warn('Failed to start a server add on', name, e); + this.serverAddOnControls.stop(name, this.device.serial).catch((e) => { + console.warn( + 'Failed to start a server add on', + name, + this.device.serial, + e, + ); }); } } diff --git a/desktop/flipper-plugin/src/plugin/Plugin.tsx b/desktop/flipper-plugin/src/plugin/Plugin.tsx index 142004a8e..553af3b0b 100644 --- a/desktop/flipper-plugin/src/plugin/Plugin.tsx +++ b/desktop/flipper-plugin/src/plugin/Plugin.tsx @@ -233,8 +233,13 @@ export class SandyPluginInstance extends BasePluginInstance { if (!this.connected.get()) { const {serverAddOn, name} = this.definition.details; if (serverAddOn) { - this.serverAddOnControls.start(name).catch((e) => { - console.warn('Failed to start a server add on', name, e); + this.serverAddOnControls.start(name, this.realClient.id).catch((e) => { + console.warn( + 'Failed to start a server add on', + name, + this.realClient.id, + e, + ); }); } @@ -248,8 +253,13 @@ export class SandyPluginInstance extends BasePluginInstance { if (this.connected.get()) { const {serverAddOn, name} = this.definition.details; if (serverAddOn) { - this.serverAddOnControls.stop(name).catch((e) => { - console.warn('Failed to stop a server add on', name, e); + this.serverAddOnControls.stop(name, this.realClient.id).catch((e) => { + console.warn( + 'Failed to stop a server add on', + name, + this.realClient.id, + e, + ); }); } diff --git a/desktop/flipper-server-core/src/FlipperServerImpl.tsx b/desktop/flipper-server-core/src/FlipperServerImpl.tsx index af20e9890..1aac53fa5 100644 --- a/desktop/flipper-server-core/src/FlipperServerImpl.tsx +++ b/desktop/flipper-server-core/src/FlipperServerImpl.tsx @@ -419,10 +419,11 @@ export class FlipperServerImpl implements FlipperServer { 'plugins-install-from-npm': (name) => this.pluginManager.installPluginFromNpm(name), 'plugin-source': (path) => this.pluginManager.loadSource(path), - 'plugins-server-add-on-start': (pluginName) => - this.pluginManager.startServerAddOn(pluginName), - 'plugins-server-add-on-stop': async (pluginName) => - this.pluginManager.stopServerAddOn(pluginName), + 'plugins-server-add-on-start': (pluginName, owner) => + this.pluginManager.startServerAddOn(pluginName, owner), + // TODO: Figure out if it needs to be async + 'plugins-server-add-on-stop': async (pluginName, owner) => + this.pluginManager.stopServerAddOn(pluginName, owner), 'doctor-get-healthchecks': getHealthChecks, 'doctor-run-healthcheck': runHealthcheck, 'open-file': openFile, diff --git a/desktop/flipper-server-core/src/comms/ServerController.tsx b/desktop/flipper-server-core/src/comms/ServerController.tsx index 3368b9ec6..b630f98a0 100644 --- a/desktop/flipper-server-core/src/comms/ServerController.tsx +++ b/desktop/flipper-server-core/src/comms/ServerController.tsx @@ -510,6 +510,7 @@ export class ServerController ); this.flipperServer.emit('client-disconnected', {id}); this.connections.delete(id); + this.flipperServer.pluginManager.stopAllServerAddOns(id); } } } diff --git a/desktop/flipper-server-core/src/devices/ServerDevice.tsx b/desktop/flipper-server-core/src/devices/ServerDevice.tsx index fd1fb6e06..3681f35ee 100644 --- a/desktop/flipper-server-core/src/devices/ServerDevice.tsx +++ b/desktop/flipper-server-core/src/devices/ServerDevice.tsx @@ -46,6 +46,7 @@ export abstract class ServerDevice { this.connected = false; this.logListener.stop(); this.crashWatcher.stop(); + this.flipperServer.pluginManager.stopAllServerAddOns(this.info.serial); } async screenshotAvailable(): Promise { diff --git a/desktop/flipper-server-core/src/plugins/PluginManager.tsx b/desktop/flipper-server-core/src/plugins/PluginManager.tsx index a92d0edc4..9f5ed424b 100644 --- a/desktop/flipper-server-core/src/plugins/PluginManager.tsx +++ b/desktop/flipper-server-core/src/plugins/PluginManager.tsx @@ -155,34 +155,39 @@ export class PluginManager { } } - async startServerAddOn(pluginName: string) { + async startServerAddOn(pluginName: string, owner: string) { console.debug('PluginManager.startServerAddOn', pluginName); - // TODO: Get owner from websocket connection ID - const fakeOwner = 'test'; const existingServerAddOn = this.serverAddOns.get(pluginName); if (existingServerAddOn) { console.debug( 'PluginManager.startServerAddOn -> already started, adding an owner', pluginName, - fakeOwner, + owner, ); - existingServerAddOn.addOwner(fakeOwner); + existingServerAddOn.addOwner(owner); return; } - const newServerAddOn = await ServerAddOn.start(pluginName, fakeOwner); + const newServerAddOn = await ServerAddOn.start(pluginName, owner, () => + this.serverAddOns.delete(pluginName), + ); this.serverAddOns.set(pluginName, newServerAddOn); } - stopServerAddOn(pluginName: string) { + stopServerAddOn(pluginName: string, owner: string) { console.debug('PluginManager.stopServerAddOn', pluginName); const serverAddOn = this.serverAddOns.get(pluginName); if (!serverAddOn) { console.debug('PluginManager.stopServerAddOn -> not started', pluginName); return; } - // TODO: Get owner from websocket connection ID - const fakeOwner = 'test'; - serverAddOn.removeOwner(fakeOwner); + serverAddOn.removeOwner(owner); + } + + stopAllServerAddOns(owner: string) { + console.debug('PluginManager.stopAllServerAddOns'); + this.serverAddOns.forEach((serverAddOn) => { + serverAddOn.removeOwner(owner); + }); } } diff --git a/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx b/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx index 1ea8cdd93..5d3ffd25b 100644 --- a/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx +++ b/desktop/flipper-server-core/src/plugins/ServerAddOn.tsx @@ -15,42 +15,49 @@ interface ServerAddOnModule { serverAddOn?: () => Promise; } -// TODO: Metro does not like dynamic requires. Figure out how to properly configure metro to handle them. -// https://github.com/webpack/webpack/issues/4175#issuecomment-323023911 -function requireDynamically(path: string) { - // eslint-disable-next-line no-eval - return eval(`require('${path}');`); // Ensure Metro does not analyze the require statement -} +const loadPlugin = (_pluginName: string): ServerAddOnModule => { + // TODO: Implement me + return {serverAddOn: async () => async () => {}}; +}; // TODO: Fix potential race conditions when starting/stopping concurrently export class ServerAddOn { private owners: Set; constructor( - public readonly path: string, + public readonly pluginName: string, private readonly cleanup: ServerAddOnCleanup, initialOwner: string, ) { this.owners = new Set(initialOwner); } - static async start(path: string, initialOwner: string): Promise { - console.info('ServerAddOn.start', path); + static async start( + pluginName: string, + initialOwner: string, + onStop: () => void, + ): Promise { + console.info('ServerAddOn.start', pluginName); - const {serverAddOn} = requireDynamically(path) as ServerAddOnModule; + const {serverAddOn} = loadPlugin(pluginName); assertNotNull(serverAddOn); assert( typeof serverAddOn === 'function', - `ServerAddOn ${path} must export "serverAddOn" function.`, + `ServerAddOn ${pluginName} must export "serverAddOn" function.`, ); const cleanup = await serverAddOn(); assert( typeof cleanup === 'function', - `ServerAddOn ${path} must return a clean up function, instead it returned ${typeof cleanup}.`, + `ServerAddOn ${pluginName} must return a clean up function, instead it returned ${typeof cleanup}.`, ); - return new ServerAddOn(path, cleanup, initialOwner); + const onStopCombined = async () => { + onStop(); + await cleanup(); + }; + + return new ServerAddOn(pluginName, onStopCombined, initialOwner); } addOwner(owner: string) { @@ -58,13 +65,13 @@ export class ServerAddOn { } removeOwner(owner: string) { - this.owners.delete(owner); + const ownerExisted = this.owners.delete(owner); - if (!this.owners.size) { + if (!this.owners.size && ownerExisted) { this.stop().catch((e) => { console.error( 'ServerAddOn.removeOwner -> failed to stop automatically when no owners left', - this.path, + this.pluginName, e, ); }); @@ -72,11 +79,11 @@ export class ServerAddOn { } private async stop() { - console.info('ServerAddOn.stop', this.path); + console.info('ServerAddOn.stop', this.pluginName); try { await this.cleanup(); } catch (e) { - console.error('ServerAddOn.stop -> failed to clean up', this.path); + console.error('ServerAddOn.stop -> failed to clean up', this.pluginName); } } } diff --git a/desktop/flipper-ui-core/src/utils/createServerAddOnControls.tsx b/desktop/flipper-ui-core/src/utils/createServerAddOnControls.tsx index 159b0e6df..623941614 100644 --- a/desktop/flipper-ui-core/src/utils/createServerAddOnControls.tsx +++ b/desktop/flipper-ui-core/src/utils/createServerAddOnControls.tsx @@ -12,8 +12,8 @@ import {FlipperServer, ServerAddOnControls} from 'flipper-common'; export const createServerAddOnControls = ( flipperServer: Pick, ): ServerAddOnControls => ({ - start: (pluginName) => - flipperServer.exec('plugins-server-add-on-start', pluginName), - stop: (pluginName) => - flipperServer.exec('plugins-server-add-on-stop', pluginName), + start: (pluginName, owner) => + flipperServer.exec('plugins-server-add-on-start', pluginName, owner), + stop: (pluginName, owner) => + flipperServer.exec('plugins-server-add-on-stop', pluginName, owner), });