Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions Composer/packages/client/src/App.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();

Expand All @@ -24,6 +25,8 @@ const { ipcRenderer } = window;
export const App: React.FC = () => {
const { appLocale } = useRecoilValue(userSettingsState);

const [isClosing, setIsClosing] = useState(false);

const {
fetchExtensions,
fetchFeatureFlags,
Expand All @@ -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 />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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
Expand Down
78 changes: 47 additions & 31 deletions Composer/packages/electron-server/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}
}
Expand Down Expand Up @@ -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'),
Expand Down Expand Up @@ -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();
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
4 changes: 2 additions & 2 deletions extensions/azurePublish/yarn-berry.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
8 changes: 7 additions & 1 deletion extensions/localPublish/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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);