fix: backend app shutdown hook is not called

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
arduino · Feb 1, 2023 · 743c9bf · 743c9bf
1 parent 31cc604
commit 743c9bf
Show file tree

Hide file tree

Showing 7 changed files with 118 additions and 104 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -14,7 +14,6 @@
         ".",
         "--log-level=debug",
         "--hostname=localhost",
-        "--no-cluster",
         "--app-project-path=${workspaceRoot}/electron-app",
         "--remote-debugging-port=9222",
         "--no-app-auto-install",
@@ -52,7 +51,6 @@
         ".",
         "--log-level=debug",
         "--hostname=localhost",
-        "--no-cluster",
         "--app-project-path=${workspaceRoot}/electron-app",
         "--remote-debugging-port=9222",
         "--no-app-auto-install",

diff --git a/arduino-ide-extension/src/browser/contributions/delete-sketch.ts b/arduino-ide-extension/src/browser/contributions/delete-sketch.ts
@@ -1,4 +1,5 @@
 import * as remote from '@theia/core/electron-shared/@electron/remote';
+import { ipcRenderer } from '@theia/core/electron-shared/electron';
 import { Dialog } from '@theia/core/lib/browser/dialogs';
 import { NavigatableWidget } from '@theia/core/lib/browser/navigatable-types';
 import { ApplicationShell } from '@theia/core/lib/browser/shell/application-shell';
@@ -9,6 +10,7 @@ import URI from '@theia/core/lib/common/uri';
 import type { Widget } from '@theia/core/shared/@phosphor/widgets';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { SketchesError } from '../../common/protocol';
+import { SCHEDULE_DELETION_SIGNAL } from '../../electron-common/electron-messages';
 import { Sketch } from '../contributions/contribution';
 import { isNotFound } from '../create/typings';
 import { Command, CommandRegistry } from './contribution';
@@ -59,7 +61,8 @@ export class DeleteSketch extends CloudSketchContribution {
       sketch = toDelete;
     }
     if (!willNavigateAway) {
-      return this.sketchesService.deleteSketch(sketch);
+      this.scheduleDeletion(sketch);
+      return;
     }
     const cloudUri = this.createFeatures.cloudUri(sketch);
     if (willNavigateAway !== 'force') {
@@ -112,10 +115,14 @@ export class DeleteSketch extends CloudSketchContribution {
       ),
     ]);
     this.windowService.setSafeToShutDown();
-    this.sketchesService.deleteSketch(sketch);
+    this.scheduleDeletion(sketch);
     return window.close();
   }
 
+  private scheduleDeletion(sketch: Sketch): void {
+    ipcRenderer.send(SCHEDULE_DELETION_SIGNAL, sketch);
+  }
+
   private async loadSketch(uri: string): Promise<Sketch | undefined> {
     try {
       const sketch = await this.sketchesService.loadSketch(uri);

diff --git a/arduino-ide-extension/src/common/protocol/sketches-service.ts b/arduino-ide-extension/src/common/protocol/sketches-service.ts
@@ -107,11 +107,6 @@ export interface SketchesService {
    */
   getIdeTempFolderUri(sketch: Sketch): Promise<string>;
 
-  /**
-   * Recursively deletes the sketch folder with all its content.
-   */
-  deleteSketch(sketch: Sketch): Promise<void>;
-
   /**
    * This is the JS/TS re-implementation of [`GenBuildPath`](https://github.com/arduino/arduino-cli/blob/c0d4e4407d80aabad81142693513b3306759cfa6/arduino/sketch/sketch.go#L296-L306) of the CLI.
    * Pass in a sketch and get the build temporary folder filesystem path calculated from the main sketch file location. Can be multiple ones. This method does not check the existence of the sketch.

diff --git a/arduino-ide-extension/src/electron-common/electron-messages.ts b/arduino-ide-extension/src/electron-common/electron-messages.ts
@@ -0,0 +1 @@
+export const SCHEDULE_DELETION_SIGNAL = 'arduino/scheduleDeletion';
diff --git a/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts b/arduino-ide-extension/src/electron-main/theia/electron-main-application.ts
@@ -9,7 +9,7 @@ import {
 import { fork } from 'child_process';
 import { AddressInfo } from 'net';
 import { join, isAbsolute, resolve } from 'path';
-import { promises as fs } from 'fs';
+import { promises as fs, rm, rmSync } from 'fs';
 import { MaybePromise } from '@theia/core/lib/common/types';
 import { ElectronSecurityToken } from '@theia/core/lib/electron-common/electron-token';
 import { FrontendApplicationConfig } from '@theia/application-package/lib/application-props';
@@ -29,6 +29,13 @@ import {
 } from '../../common/ipc-communication';
 import { ErrnoException } from '../../node/utils/errors';
 import { isAccessibleSketchPath } from '../../node/sketches-service-impl';
+import { SCHEDULE_DELETION_SIGNAL } from '../../electron-common/electron-messages';
+import { FileUri } from '@theia/core/lib/node/file-uri';
+import {
+  Disposable,
+  DisposableCollection,
+} from '@theia/core/lib/common/disposable';
+import { Sketch } from '../../common/protocol';
 
 app.commandLine.appendSwitch('disable-http-cache');
 
@@ -66,6 +73,34 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
   private startup = false;
   private _firstWindowId: number | undefined;
   private openFilePromise = new Deferred();
+  /**
+   * It contains all things the IDE2 must clean up before a normal stop.
+   *
+   * When deleting the sketch, the IDE2 must close the browser window and
+   * recursively delete the sketch folder from the filesystem. The sketch
+   * cannot be deleted when the window is open because that is the currently
+   * opened workspace. IDE2 cannot delete the sketch folder from the
+   * filesystem after closing the browser window because the window can be
+   * the last, and when the last window closes, the application quits.
+   * There is no way to clean up the undesired resources.
+   *
+   * This array contains disposable instances wrapping synchronous sketch
+   * delete operations. When IDE2 closes the browser window, it schedules
+   * the sketch deletion, and the window closes.
+   *
+   * When IDE2 schedules a sketch for deletion, it creates a synchronous
+   * folder deletion as a disposable instance and pushes it into this
+   * array. After the push, IDE2 starts the sketch deletion in an
+   * asynchronous way. When the deletion completes, the disposable is
+   * removed. If the app quits when the asynchronous deletion is still in
+   * progress, it disposes the elements of this array. Since it is
+   * synchronous, it is [ensured by Theia](https://github.com/eclipse-theia/theia/blob/678e335644f1b38cb27522cc27a3b8209293cf31/packages/core/src/node/backend-application.ts#L91-L97)
+   * that IDE2 won't quit before the cleanup is done. It works only in normal
+   * quit.
+   */
+  // TODO: Why is it here and not in the Theia backend?
+  // https://github.com/eclipse-theia/theia/discussions/12135
+  private readonly scheduledDeletions: Disposable[] = [];
 
   override async start(config: FrontendApplicationConfig): Promise<void> {
     // Explicitly set the app name to have better menu items on macOS. ("About", "Hide", and "Quit")
@@ -309,6 +344,13 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
     ipcMain.on(Restart, ({ sender }) => {
       this.restart(sender.id);
     });
+    ipcMain.on(SCHEDULE_DELETION_SIGNAL, (event, sketch: unknown) => {
+      if (Sketch.is(sketch)) {
+        console.log(`Sketch ${sketch.uri} was scheduled for deletion`);
+        // TODO: remove deleted sketch from closedWorkspaces?
+        this.delete(sketch);
+      }
+    });
   }
 
   protected override async onSecondInstance(
@@ -511,6 +553,16 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
       `Stored workspaces roots: ${workspaces.map(({ file }) => file)}`
     );
 
+    if (this.scheduledDeletions.length) {
+      console.log(
+        '>>> Finishing scheduled sketch deletions before app quit...'
+      );
+      new DisposableCollection(...this.scheduledDeletions).dispose();
+      console.log('<<< Successfully finishing scheduled sketch deletions.');
+    } else {
+      console.log('No sketches were scheduled for deletion.');
+    }
+
     super.onWillQuit(event);
   }
 
@@ -521,6 +573,59 @@ export class ElectronMainApplication extends TheiaElectronMainApplication {
   get firstWindowId(): number | undefined {
     return this._firstWindowId;
   }
+
+  private async delete(sketch: Sketch): Promise<void> {
+    const sketchPath = FileUri.fsPath(sketch.uri);
+    const disposable = Disposable.create(() => {
+      try {
+        this.deleteSync(sketchPath);
+      } catch (err) {
+        console.error(
+          `Could not delete sketch ${sketchPath} on app quit.`,
+          err
+        );
+      }
+    });
+    this.scheduledDeletions.push(disposable);
+    return new Promise<void>((resolve, reject) => {
+      rm(sketchPath, { recursive: true, maxRetries: 5 }, (error) => {
+        if (error) {
+          console.error(`Failed to delete sketch ${sketchPath}`, error);
+          reject(error);
+        } else {
+          console.info(`Successfully deleted sketch ${sketchPath}`);
+          resolve();
+          const index = this.scheduledDeletions.indexOf(disposable);
+          if (index >= 0) {
+            this.scheduledDeletions.splice(index, 1);
+            console.info(
+              `Successfully completed the scheduled sketch deletion: ${sketchPath}`
+            );
+          } else {
+            console.warn(
+              `Could not find the scheduled sketch deletion: ${sketchPath}`
+            );
+          }
+        }
+      });
+    });
+  }
+
+  private deleteSync(sketchPath: string): void {
+    console.info(
+      `>>> Running sketch deletion ${sketchPath} before app quit...`
+    );
+    try {
+      rmSync(sketchPath, { recursive: true, maxRetries: 5 });
+      console.info(`<<< Deleted sketch ${sketchPath}`);
+    } catch (err) {
+      if (!ErrnoException.isENOENT(err)) {
+        throw err;
+      } else {
+        console.info(`<<< Sketch ${sketchPath} did not exist.`);
+      }
+    }
+  }
 }
 
 class InterruptWorkspaceRestoreError extends Error {

diff --git a/arduino-ide-extension/src/node/arduino-ide-backend-module.ts b/arduino-ide-extension/src/node/arduino-ide-backend-module.ts
@@ -196,7 +196,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
   // Shared sketches service
   bind(SketchesServiceImpl).toSelf().inSingletonScope();
   bind(SketchesService).toService(SketchesServiceImpl);
-  bind(BackendApplicationContribution).toService(SketchesServiceImpl);
   bind(ConnectionHandler)
     .toDynamicValue(
       (context) =>

diff --git a/arduino-ide-extension/src/node/sketches-service-impl.ts b/arduino-ide-extension/src/node/sketches-service-impl.ts
@@ -1,13 +1,5 @@
 import { injectable, inject, named } from '@theia/core/shared/inversify';
-import {
-  promises as fs,
-  realpath,
-  lstat,
-  Stats,
-  constants,
-  rm,
-  rmSync,
-} from 'fs';
+import { promises as fs, realpath, lstat, Stats, constants } from 'fs';
 import * as os from 'os';
 import * as temp from 'temp';
 import * as path from 'path';
@@ -51,11 +43,6 @@ import {
   firstToUpperCase,
   startsWithUpperCase,
 } from '../common/utils';
-import { BackendApplicationContribution } from '@theia/core/lib/node/backend-application';
-import {
-  Disposable,
-  DisposableCollection,
-} from '@theia/core/lib/common/disposable';
 
 const RecentSketches = 'recent-sketches.json';
 const DefaultIno = `void setup() {
@@ -72,7 +59,7 @@ void loop() {
 @injectable()
 export class SketchesServiceImpl
   extends CoreClientAware
-  implements SketchesService, BackendApplicationContribution
+  implements SketchesService
 {
   private sketchSuffixIndex = 1;
   private lastSketchBaseName: string;
@@ -82,32 +69,6 @@ export class SketchesServiceImpl
     concurrency: 1,
   });
   private inoContent: Deferred<string> | undefined;
-  /**
-   * It contains all things the IDE2 must clean up before a normal stop.
-   *
-   * When deleting the sketch, the IDE2 must close the browser window and
-   * recursively delete the sketch folder from the filesystem. The sketch
-   * cannot be deleted when the window is open because that is the currently
-   * opened workspace. IDE2 cannot delete the sketch folder from the
-   * filesystem after closing the browser window because the window can be
-   * the last, and when the last window closes, the application quits.
-   * There is no way to clean up the undesired resources.
-   *
-   * This array contains disposable instances wrapping synchronous sketch
-   * delete operations. When IDE2 closes the browser window, it schedules
-   * the sketch deletion, and the window closes.
-   *
-   * When IDE2 schedules a sketch for deletion, it creates a synchronous
-   * folder deletion as a disposable instance and pushes it into this
-   * array. After the push, IDE2 starts the sketch deletion in an
-   * asynchronous way. When the deletion completes, the disposable is
-   * removed. If the app quits when the asynchronous deletion is still in
-   * progress, it disposes the elements of this array. Since it is
-   * synchronous, it is [ensured by Theia](https://github.com/eclipse-theia/theia/blob/678e335644f1b38cb27522cc27a3b8209293cf31/packages/core/src/node/backend-application.ts#L91-L97)
-   * that IDE2 won't quit before the cleanup is done. It works only in normal
-   * quit.
-   */
-  private readonly scheduledDeletions: Disposable[] = [];
 
   @inject(ILogger)
   @named('sketches-service')
@@ -125,14 +86,6 @@ export class SketchesServiceImpl
   @inject(IsTempSketch)
   private readonly isTempSketch: IsTempSketch;
 
-  onStop(): void {
-    if (this.scheduledDeletions.length) {
-      this.logger.info(`>>> Disposing sketches service...`);
-      new DisposableCollection(...this.scheduledDeletions).dispose();
-      this.logger.info(`<<< Disposed sketches service.`);
-    }
-  }
-
   async getSketches({ uri }: { uri?: string }): Promise<SketchContainer> {
     const root = await this.root(uri);
     if (!root) {
@@ -679,50 +632,6 @@ export class SketchesServiceImpl
     return folderName;
   }
 
-  async deleteSketch(sketch: Sketch): Promise<void> {
-    const sketchPath = FileUri.fsPath(sketch.uri);
-    const disposable = Disposable.create(() =>
-      this.deleteSketchSync(sketchPath)
-    );
-    this.scheduledDeletions.push(disposable);
-    return new Promise<void>((resolve, reject) => {
-      rm(sketchPath, { recursive: true, maxRetries: 5 }, (error) => {
-        if (error) {
-          this.logger.error(`Failed to delete sketch at ${sketchPath}.`, error);
-          reject(error);
-        } else {
-          this.logger.info(`Successfully deleted sketch at ${sketchPath}.`);
-          resolve();
-          const index = this.scheduledDeletions.indexOf(disposable);
-          if (index >= 0) {
-            this.scheduledDeletions.splice(index, 1);
-            this.logger.info(
-              `Removed the successfully completed scheduled sketch deletion: ${sketchPath}`
-            );
-          } else {
-            this.logger.warn(
-              `Could not find the scheduled sketch deletion: ${sketchPath}`
-            );
-          }
-        }
-      });
-    });
-  }
-
-  private deleteSketchSync(sketchPath: string): void {
-    this.logger.info(
-      `>>> Running sketch deletion ${sketchPath} before app quit...`
-    );
-    try {
-      rmSync(sketchPath, { recursive: true, maxRetries: 5 });
-      this.logger.info(`<<< Deleted sketch ${sketchPath}.`);
-    } catch (err) {
-      if (!ErrnoException.isENOENT(err)) {
-        throw err;
-      }
-    }
-  }
-
   // Returns the default.ino from the settings or from default folder.
   private async readSettings(): Promise<Record<string, unknown> | undefined> {
     const configDirUri = await this.envVariableServer.getConfigDirUri();
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		export const SCHEDULE_DELETION_SIGNAL = 'arduino/scheduleDeletion';