Merge branch 'unsubscribe-from-daemon-events-on-exit'

4 files changed, 73 insertions, 13 deletions

diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts
index 4e349aae36..481833e0ac 100644
--- a/gui/src/main/daemon-rpc.ts
+++ b/gui/src/main/daemon-rpc.ts
@@ -303,6 +303,10 @@ export class ConnectionObserver {
 }
 
 export class SubscriptionListener<T> {
+  // Only meant to be used by DaemonRpc
+  // @internal
+  public subscriptionId?: string | number;
+
   constructor(
     private eventHandler: (payload: T) => void,
     private errorHandler: (error: Error) => void,
@@ -493,17 +497,31 @@ export class DaemonRpc {
     }
   }
 
-  public subscribeDaemonEventListener(listener: SubscriptionListener<DaemonEvent>): Promise<void> {
-    return this.transport.subscribe('daemon_event', (payload) => {
+  public async subscribeDaemonEventListener(
+    listener: SubscriptionListener<DaemonEvent>,
+  ): Promise<void> {
+    const subscriptionId = await this.transport.subscribe('daemon_event', (payload) => {
+      let daemonEvent: DaemonEvent;
+
       try {
-        const daemonEvent = camelCaseObjectKeys(
-          validate(daemonEventSchema, payload),
-        ) as DaemonEvent;
-        listener.onEvent(daemonEvent);
+        daemonEvent = camelCaseObjectKeys(validate(daemonEventSchema, payload)) as DaemonEvent;
       } catch (error) {
         listener.onError(new ResponseParseError('Invalid payload from daemon_event', error));
+        return;
       }
+
+      listener.onEvent(daemonEvent);
     });
+
+    listener.subscriptionId = subscriptionId;
+  }
+
+  public async unsubscribeDaemonEventListener(
+    listener: SubscriptionListener<DaemonEvent>,
+  ): Promise<void> {
+    if (listener.subscriptionId) {
+      return this.transport.unsubscribe('daemon_event', listener.subscriptionId);
+    }
   }
 
   public async getAccountHistory(): Promise<AccountToken[]> {
diff --git a/gui/src/main/index.ts b/gui/src/main/index.ts
index d7e13615f9..4d491a9cc8 100644
--- a/gui/src/main/index.ts
+++ b/gui/src/main/index.ts
@@ -74,6 +74,7 @@ class ApplicationMain {
   private trayIconController?: TrayIconController;
 
   private daemonRpc = new DaemonRpc();
+  private daemonEventListener?: SubscriptionListener<DaemonEvent>;
   private reconnectBackoff = new ReconnectionBackoff();
   private connectedToDaemon = false;
   private quitStage = AppQuitStage.unready;
@@ -271,6 +272,17 @@ class ApplicationMain {
       log.info('Cannot close the tunnel because there is no active connection to daemon.');
     }
 
+    // Unsubscribe the event handler
+    try {
+      if (this.daemonEventListener) {
+        await this.daemonRpc.unsubscribeDaemonEventListener(this.daemonEventListener);
+
+        log.info('Unsubscribed from the daemon events');
+      }
+    } catch (e) {
+      log.error(`Failed to unsubscribe from daemon events: ${e.message}`);
+    }
+
     // The window is not closable on macOS to be able to hide the titlebar and workaround
     // a shadow bug rendered above the invisible title bar. This also prevents the window from
     // closing normally, even programmatically. Therefore re-enable the close button just before
@@ -375,7 +387,7 @@ class ApplicationMain {
 
     // subscribe to events
     try {
-      await this.subscribeEvents();
+      this.daemonEventListener = await this.subscribeEvents();
     } catch (error) {
       log.error(`Failed to subscribe: ${error.message}`);
 
@@ -456,6 +468,9 @@ class ApplicationMain {
     // connection loss.
     const wasConnected = this.connectedToDaemon;
 
+    // Reset the daemon event listener since it's going to be invalidated on disconnect
+    this.daemonEventListener = undefined;
+
     if (wasConnected) {
       this.connectedToDaemon = false;
 
@@ -506,7 +521,7 @@ class ApplicationMain {
     this.reconnectToDaemon();
   }
 
-  private async subscribeEvents(): Promise<void> {
+  private async subscribeEvents(): Promise<SubscriptionListener<DaemonEvent>> {
     const daemonEventListener = new SubscriptionListener(
       (daemonEvent: DaemonEvent) => {
         if ('tunnelState' in daemonEvent) {
@@ -528,7 +543,9 @@ class ApplicationMain {
       },
     );
 
-    return this.daemonRpc.subscribeDaemonEventListener(daemonEventListener);
+    await this.daemonRpc.subscribeDaemonEventListener(daemonEventListener);
+
+    return daemonEventListener;
   }
 
   private setAccountHistory(accountHistory: AccountToken[]) {
@@ -1311,7 +1328,6 @@ class ApplicationMain {
   // setup NSEvent monitor to fix inconsistent window.blur on macOS
   // see https://github.com/electron/electron/issues/8689
   private installMacOsMenubarAppWindowHandlers(tray: Tray, windowController: WindowController) {
-    // $FlowFixMe: this module is only available on macOS
     const { NSEventMonitor, NSEventMask } = require('nseventmonitor');
     const macEventMonitor = new NSEventMonitor();
     // tslint:disable-next-line
diff --git a/gui/src/main/jsonrpc-client.ts b/gui/src/main/jsonrpc-client.ts
index f3ac7e78e1..357d085a14 100644
--- a/gui/src/main/jsonrpc-client.ts
+++ b/gui/src/main/jsonrpc-client.ts
@@ -163,11 +163,12 @@ export default class JsonRpcClient<T> extends EventEmitter {
     }
   }
 
-  public async subscribe(event: string, listener: (value: any) => void): Promise<void> {
+  public async subscribe(event: string, listener: (value: any) => void): Promise<string | number> {
     log.silly(`Adding a listener for ${event}`);
 
     try {
       const subscriptionId = await this.send(`${event}_subscribe`);
+
       if (typeof subscriptionId === 'string' || typeof subscriptionId === 'number') {
         this.subscriptions.set(subscriptionId, listener);
       } else {
@@ -176,12 +177,29 @@ export default class JsonRpcClient<T> extends EventEmitter {
           subscriptionId,
         );
       }
+
+      return subscriptionId;
     } catch (e) {
       log.error(`Failed adding listener to ${event}: ${e.message}`);
       throw e;
     }
   }
 
+  public async unsubscribe(event: string, subscriptionId: string | number): Promise<void> {
+    log.silly(`Removing a listener for ${event}`);
+
+    try {
+      if (this.subscriptions.has(subscriptionId)) {
+        await this.send(`${event}_unsubscribe`, [subscriptionId]);
+      }
+    } catch (e) {
+      log.error(`Failed removing listener to ${event}: ${e.message}`);
+      throw e;
+    } finally {
+      this.subscriptions.delete(subscriptionId);
+    }
+  }
+
   public send(action: string, data?: any, timeout: number = DEFAULT_TIMEOUT_MILLIS): Promise<any> {
     return new Promise((resolve, reject) => {
       const transport = this.transport;
diff --git a/gui/src/shared/ipc-event-channel.ts b/gui/src/shared/ipc-event-channel.ts
index a11d41bf10..f57368db2d 100644
--- a/gui/src/shared/ipc-event-channel.ts
+++ b/gui/src/shared/ipc-event-channel.ts
@@ -361,13 +361,21 @@ function set<T>(event: string): (value: T) => void {
 
 function sender<T>(event: string): (webContents: WebContents, value: T) => void {
   return (webContents: WebContents, value: T) => {
-    webContents.send(event, value);
+    if (webContents.isDestroyed()) {
+      log.error(`sender(${event}): webContents is already destroyed!`);
+    } else {
+      webContents.send(event, value);
+    }
   };
 }
 
 function senderVoid(event: string): (webContents: WebContents) => void {
   return (webContents: WebContents) => {
-    webContents.send(event);
+    if (webContents.isDestroyed()) {
+      log.error(`senderVoid(${event}): webContents is already destroyed!`);
+    } else {
+      webContents.send(event);
+    }
   };
 }