Skip to content

Commit c070518

Browse files
committed
Improve deactivate method to ensure more cleanup fires when exceptions occur
The calls to setContext() can throw during shutdown, which could result in some shutdown being skipped. This should be more resilient. See #5155
1 parent decff41 commit c070518

2 files changed

Lines changed: 51 additions & 28 deletions

File tree

‎src/extension/extension.ts‎

Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { DartCapabilities } from "../shared/capabilities/dart";
55
import { DaemonCapabilities, FlutterCapabilities } from "../shared/capabilities/flutter";
66
import { vsCodeVersion } from "../shared/capabilities/vscode";
77
import { dartPlatformName, flutterExtensionIdentifier, isDartCodeTestRun, isMac, isWin, platformDisplayName } from "../shared/constants";
8-
import { DART_PLATFORM_NAME, DART_PROJECT_LOADED, FLUTTER_PROJECT_LOADED, FLUTTER_SIDEBAR_SUPPORTED_CONTEXT, FLUTTER_SUPPORTS_ATTACH, HAS_LAST_DEBUG_CONFIG, HAS_LAST_TEST_DEBUG_CONFIG, IS_LSP_CONTEXT, IS_RUNNING_LOCALLY_CONTEXT, PROJECT_LOADED, PUB_OUTDATED_SUPPORTED_CONTEXT, SDK_IS_PRE_RELEASE, WEB_PROJECT_LOADED } from "../shared/constants.contexts";
8+
import { DART_PLATFORM_NAME, DART_PROJECT_LOADED, FLUTTER_PROJECT_LOADED, FLUTTER_SIDEBAR_SUPPORTED_CONTEXT, FLUTTER_SUPPORTS_ATTACH, IS_LSP_CONTEXT, IS_RUNNING_LOCALLY_CONTEXT, PROJECT_LOADED, PUB_OUTDATED_SUPPORTED_CONTEXT, SDK_IS_PRE_RELEASE, WEB_PROJECT_LOADED } from "../shared/constants.contexts";
99
import { LogCategory } from "../shared/enums";
1010
import { WebClient } from "../shared/fetch";
1111
import { DartWorkspaceContext, FlutterSdks, FlutterWorkspaceContext, IAmDisposable, IFlutterDaemon, Logger, Sdks, WritableWorkspaceConfig } from "../shared/interfaces";
@@ -139,6 +139,7 @@ let experiments: KnownExperiments;
139139
const loggers: IAmDisposable[] = [];
140140
let ringLogger: IAmDisposable | undefined;
141141
const logger = new EmittingLogger();
142+
let extensionLog: IAmDisposable | undefined;
142143

143144
// Keep a running in-memory buffer of last 200 log events we can give to the
144145
// user when something crashed even if they don't have disk-logging enabled.
@@ -154,7 +155,8 @@ export async function activate(context: vs.ExtensionContext, isRestart = false)
154155

155156
void vs.commands.executeCommand("setContext", IS_RUNNING_LOCALLY_CONTEXT, isRunningLocally);
156157
buildLogHeaders();
157-
setupLog(getExtensionLogPath(), LogCategory.General);
158+
if (!extensionLog)
159+
extensionLog = setupLog(getExtensionLogPath(), LogCategory.General, false);
158160

159161
const webClient = new WebClient(extensionVersion);
160162

@@ -869,9 +871,13 @@ export async function activate(context: vs.ExtensionContext, isRestart = false)
869871
// }
870872
}
871873

872-
function setupLog(logFile: string | undefined, category: LogCategory) {
873-
if (logFile)
874-
loggers.push(captureLogs(logger, logFile, getLogHeader(), config.maxLogLineLength, [category]));
874+
function setupLog(logFile: string | undefined, category: LogCategory, autoDispose = true) {
875+
if (logFile) {
876+
const fileLogger = captureLogs(logger, logFile, getLogHeader(), config.maxLogLineLength, [category]);
877+
if (autoDispose)
878+
loggers.push(fileLogger);
879+
return fileLogger;
880+
}
875881
}
876882

877883
function buildLogHeaders(logger?: Logger, workspaceContext?: WorkspaceContext) {
@@ -999,28 +1005,31 @@ function getSettingsThatRequireRestart() {
9991005

10001006
export async function deactivate(isRestart = false): Promise<void> {
10011007
logger.info(`Extension deactivate was called (isRestart: ${isRestart})`);
1002-
try {
1003-
setCommandVisiblity(false);
1004-
void analyzer?.dispose();
1005-
await flutterDaemon?.shutdown();
1006-
if (loggers) {
1007-
await Promise.all(loggers.map((logger) => logger.dispose()));
1008-
loggers.length = 0;
1009-
}
1010-
void vs.commands.executeCommand("setContext", FLUTTER_SUPPORTS_ATTACH, false);
1011-
if (!isRestart) {
1012-
void vs.commands.executeCommand("setContext", HAS_LAST_DEBUG_CONFIG, false);
1013-
void vs.commands.executeCommand("setContext", HAS_LAST_TEST_DEBUG_CONFIG, false);
1014-
void ringLogger?.dispose();
1015-
logger.dispose();
1016-
} else {
1017-
// If we are starting, add an additional delay just to make it more likely other services
1018-
// have shut down before we try to restart them.
1019-
await new Promise((resolve) => setTimeout(resolve, 500));
1020-
}
1021-
logger.info(`Extension deactivated!`);
1022-
} catch (e) {
1023-
logger.error(`Error during extension shutdown: ${e}`);
1008+
1009+
await Promise.allSettled([
1010+
tryCleanup(() => setCommandVisiblity(false)),
1011+
tryCleanup(() => analyzer?.dispose()),
1012+
tryCleanup(() => flutterDaemon?.shutdown()),
1013+
tryCleanup(() => vs.commands.executeCommand("setContext", FLUTTER_SUPPORTS_ATTACH, false)),
1014+
...loggers.map((l) => tryCleanup(() => l.dispose())),
1015+
]);
1016+
logger.info(`Extension cleanup done`);
1017+
1018+
// Pump for any log events that might need to be written to the loggers.
1019+
await new Promise((resolve) => setTimeout(resolve, 100));
1020+
1021+
if (!isRestart) {
1022+
logger.info(`Closing all loggers...`);
1023+
await new Promise((resolve) => setTimeout(resolve, 50));
1024+
await Promise.allSettled([
1025+
tryCleanup(() => logger.dispose()),
1026+
tryCleanup(() => ringLogger?.dispose()),
1027+
tryCleanup(() => extensionLog?.dispose()),
1028+
]);
1029+
await new Promise((resolve) => setTimeout(resolve, 50));
1030+
} else {
1031+
logger.info(`Restarting...`);
1032+
await new Promise((resolve) => setTimeout(resolve, 300));
10241033
}
10251034
}
10261035

@@ -1031,3 +1040,16 @@ function setCommandVisiblity(enable: boolean, workspaceContext?: WorkspaceContex
10311040
void vs.commands.executeCommand("setContext", WEB_PROJECT_LOADED, enable && workspaceContext && workspaceContext.hasAnyWebProjects);
10321041
}
10331042

1043+
/// Calls a cleanup function in a try/catch to ensure we never throw and logs any error to the logger
1044+
/// and the console.
1045+
async function tryCleanup(f: () => void | Promise<void> | Thenable<void>): Promise<void> {
1046+
try {
1047+
await f();
1048+
} catch (e) {
1049+
try {
1050+
console.error(`Error cleaning up during extension shutdown: ${e}`);
1051+
logger.error(`Error cleaning up during extension shutdown: ${e}`);
1052+
} catch { }
1053+
}
1054+
}
1055+

‎src/shared/logging.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ export function captureLogs(logger: EmittingLogger, file: string, header: string
147147
return new Promise((resolve) => {
148148
if (logStream) {
149149
logStream.write(`${(new Date()).toDateString()} ${time(true)}Log file ended${os.EOL}`);
150-
logStream.end(resolve);
150+
logStream.once("finish", resolve);
151+
logStream.end();
151152
logStream = undefined;
152153
}
153154
});

0 commit comments

Comments
 (0)