fix: rework app exit to account for running bots (#9532)

* Work around kill on windows doesn't kill process with shell:true

* Proper app teardown

* fix windows not able to call taskkill without shell

* Fix bot cleanup process

* Rework app exit flow

* flushExistingTasks to properly call Promise.all

* Re-initialize close listeners on app reopened on Mac

* allow user to hide window while teardown happens

* Refactor and improve

* Do not prevent subsequent attempts to close the main window

* fix: possible race condition for window recreation on mac

* minor improvements

* simplify exit logic

* bring back missing destroy call

* fix flaky test

* fix lock

---------

Co-authored-by: sw-joelmut <joel.mut@southworks.com>

Composer/packages/client/src/App.tsx

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import React, { Fragment, useEffect } from 'react';
+import React, { Fragment, useEffect, useState } from 'react';
 import { useRecoilValue } from 'recoil';
 
 import { Header } from './components/Header';
@@ -12,6 +12,7 @@ import { loadLocale } from './utils/fileUtil';
 import { useInitializeLogger } from './telemetry/useInitializeLogger';
 import { setupIcons } from './setupIcons';
 import { setOneAuthEnabled } from './utils/oneAuthUtil';
+import { LoadingSpinner } from './components/LoadingSpinner';
 
 setupIcons();
 
@@ -24,6 +25,8 @@ const { ipcRenderer } = window;
 export const App: React.FC = () => {
   const { appLocale } = useRecoilValue(userSettingsState);
 
+  const [isClosing, setIsClosing] = useState(false);
+
   const {
     fetchExtensions,
     fetchFeatureFlags,
@@ -40,18 +43,22 @@ export const App: React.FC = () => {
     checkNodeVersion();
     fetchExtensions();
     fetchFeatureFlags();
-    ipcRenderer?.on('cleanup', (_event) => {
-      performAppCleanupOnQuit();
-    });
 
     ipcRenderer?.invoke('app-init').then(({ machineInfo, isOneAuthEnabled }) => {
       setMachineInfo(machineInfo);
       setOneAuthEnabled(isOneAuthEnabled);
     });
+
+    ipcRenderer?.on('closing', async () => {
+      setIsClosing(true);
+      await performAppCleanupOnQuit();
+      ipcRenderer.send('closed');
+    });
   }, []);
 
   return (
     <Fragment key={appLocale}>
+      {isClosing && <LoadingSpinner inModal message="Finishing closing the application. Performing cleanup." />}
       <Logger />
       <Announcement />
       <Header />

Composer/packages/client/src/recoilModel/dispatchers/utils/project.ts

@@ -103,7 +103,7 @@ import { getPublishProfileFromPayload } from '../../../utils/electronUtil';
 import { crossTrainConfigState, projectIndexingState } from './../../atoms/botState';
 import { recognizersSelectorFamily } from './../../selectors/recognizers';
 
-export const resetBotStates = async ({ reset }: CallbackInterface, projectId: string) => {
+export const resetBotStates = ({ reset }: CallbackInterface, projectId: string) => {
   const botStates = Object.keys(botstates);
   botStates.forEach((state) => {
     const currentRecoilAtom: any = botstates[state];
@@ -137,13 +137,15 @@ export const flushExistingTasks = async (callbackHelpers: CallbackInterface) =>
   reset(botProjectSpaceLoadedState);
   reset(botProjectIdsState);
 
-  for (const projectId of projectIds) {
-    botRuntimeOperations?.stopBot(projectId);
+  const result = projectIds.map(async (projectId) => {
+    await botRuntimeOperations?.stopBot(projectId);
     resetBotStates(callbackHelpers, projectId);
-  }
+  });
 
-  const workers = [lgWorker, luWorker, qnaWorker];
-  return Promise.all([workers.map((w) => w.flush())]);
+  const workers = [lgWorker, luWorker, qnaWorker].map(async (worker) => {
+    await worker.flush();
+  });
+  await Promise.all([...result, ...workers]);
 };
 
 // merge sensitive values in localStorage

Composer/packages/electron-server/src/main.ts

@@ -209,10 +209,6 @@ async function main(show = false) {
           console.error('[Windows] Error while waiting for main window to show before processing deep link: ', e)
         );
     }
-
-    mainWindow.on('closed', () => {
-      ElectronWindow.destroy();
-    });
     log('Rendered application.');
   }
 }
@@ -269,13 +265,55 @@ async function run() {
     app.quit();
   }
 
+  const getMainWindow = () => ElectronWindow.getInstance().browserWindow;
+
+  const initApp = async () => {
+    let mainWindow = getMainWindow();
+    if (!mainWindow) return;
+
+    mainWindow.webContents.send('session-update', 'session-started');
+
+    if (process.env.COMPOSER_DEV_TOOLS) {
+      mainWindow.webContents.openDevTools();
+    }
+
+    mainWindow.on('close', (event) => {
+      // when the window is not visible, it means that window.close
+      // has been called by the handler, so it is time to proceed
+      if (!mainWindow?.isVisible) {
+        return;
+      }
+
+      event.preventDefault();
+      mainWindow.hide();
+      mainWindow.webContents.send('session-update', 'session-ended');
+
+      // Give 30 seconds to close app gracefully, then proceed
+      Promise.race([
+        new Promise<void>((resolve) => setTimeout(resolve, 30000)),
+        new Promise<void>((resolve) => ipcMain.once('closed', () => resolve())),
+      ]).then(() => {
+        mainWindow?.close();
+        mainWindow = undefined;
+        ElectronWindow.destroy();
+
+        // preserve app icon in the dock on MacOS
+        if (isMac()) return;
+
+        process.emit('beforeExit', 0);
+        app.quit();
+      });
+
+      mainWindow.webContents.send('closing');
+    });
+  };
+
   app.on('ready', async () => {
     log('App ready');
 
     log('Loading latest known locale');
     loadLocale(currentAppLocale);
 
-    const getMainWindow = () => ElectronWindow.getInstance().browserWindow;
     const { startApp, updateStatus } = await initSplashScreen({
       getMainWindow,
       icon: join(__dirname, '../resources/composerIcon_1024x1024.png'),
@@ -306,38 +344,16 @@ async function run() {
     });
 
     await main();
-
     setTimeout(() => startApp(signalThatMainWindowIsShowing), 500);
-
-    const mainWindow = getMainWindow();
-
-    mainWindow?.webContents.send('session-update', 'session-started');
-
-    if (process.env.COMPOSER_DEV_TOOLS) {
-      mainWindow?.webContents.openDevTools();
-    }
-  });
-
-  // Quit when all windows are closed.
-  app.on('window-all-closed', () => {
-    // On OS X it is common for applications and their menu bar
-    // to stay active until the user quits explicitly with Cmd + Q
-    if (!isMac()) {
-      app.quit();
-    }
-  });
-
-  app.on('before-quit', () => {
-    const mainWindow = ElectronWindow.getInstance().browserWindow;
-    mainWindow?.webContents.send('session-update', 'session-ended');
-    mainWindow?.webContents.send('cleanup');
+    await initApp();
   });
 
-  app.on('activate', () => {
+  app.on('activate', async () => {
     // On OS X it's common to re-create a window in the app when the
     // dock icon is clicked and there are no other windows open.
     if (!ElectronWindow.isBrowserWindowCreated) {
-      main(true);
+      await main(true);
+      initApp();
     }
   });
 

Composer/packages/lib/code-editor/src/components/toolbar/__tests__/ToolbarButtonMenu.test.tsx

@@ -117,7 +117,7 @@ describe('<ToolbarButtonMenu />', () => {
       jest.runAllTimers();
     });
 
-    expect((await screen.findAllByText(/this/)).length).toBe(3);
+    expect((await screen.findAllByText(/this\.\w/)).length).toBe(2);
   });
 
   it('property: Should expand property in the menu on click if not leaf', async () => {

extensions/azurePublish/yarn-berry.lock

@@ -1852,7 +1852,7 @@ __metadata:
 
 "@bfc/code-editor@file:../../Composer/packages/lib/code-editor::locator=azurePublish%40workspace%3A.":
   version: 0.0.0
-  resolution: "@bfc/code-editor@file:../../Composer/packages/lib/code-editor#../../Composer/packages/lib/code-editor::hash=85aa6e&locator=azurePublish%40workspace%3A."
+  resolution: "@bfc/code-editor@file:../../Composer/packages/lib/code-editor#../../Composer/packages/lib/code-editor::hash=370616&locator=azurePublish%40workspace%3A."
   dependencies:
     "@emotion/react": ^11.1.3
     "@emotion/styled": ^11.1.3
@@ -1875,7 +1875,7 @@ __metadata:
     "@bfc/ui-shared": "*"
     react: 16.13.1
     react-dom: 16.13.1
-  checksum: 29f5412bdf1777648742441cb612fd4809132e7a1fe595e18f9f7911e167a66e53022254ababf281168d0b48d27d6c1ec7209e29d280633fe80189538aec9606
+  checksum: 0641044374678271252eb2b5e6707068cae2a8ed9359ebf03d28a0075ff146c80321018c9725f17cdd3d16caed6cb890fede89cc98aa54a9fadd8bf6e4ccff11
   languageName: node
   linkType: hard
 

extensions/localPublish/src/index.ts

@@ -458,7 +458,11 @@ class LocalPublisher implements PublishPlugin<PublishConfig> {
       const bot = LocalPublisher.runningBots[botId];
       // Kill the bot process AND all child processes
       try {
-        process.kill(isWin ? bot.process.pid : -bot.process.pid);
+        if (isWin) {
+          spawn(`taskkill /pid ${bot.process.pid} /T /F`, { shell: true });
+        } else {
+          process.kill(-bot.process.pid);
+        }
       } catch (err) {
         // swallow this error which happens if the child process is already gone
       }
@@ -482,3 +486,5 @@ const cleanup = () => {
 (['SIGINT', 'SIGTERM', 'SIGQUIT'] as NodeJS.Signals[]).forEach((signal: NodeJS.Signals) => {
   process.on(signal, cleanup.bind(null, signal));
 });
+
+process.on('beforeExit', cleanup);