Skip to content

Commit ef5c543

Browse files
committed
when session cache is used, register cleanup at tempContext cleanup and not exit cleanup
It happens we do cleanup in several places, with at least - renderServices cleanup calling `lifetime.cleanup()` - exit cleanup calling `exitWithCleanup()` The problem is that when session cache is used, we need to register cleanup at tempContext cleanup and not just exit cleanup because tempContext cleanup will try to remove the directory in session cache, and it will fail because Deno.kv is still opened.
1 parent 067171f commit ef5c543

File tree

4 files changed

+29
-9
lines changed

4 files changed

+29
-9
lines changed

src/core/sass.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,8 @@ export async function compileWithCache(
307307
const cacheDir = useSessionCache
308308
? join(temp.baseDir, "sass")
309309
: quartoCacheDir("sass");
310-
// when using quarto session cache, we ensure to cleanup the cache files
311-
const cache = await sassCache(cacheDir, !!useSessionCache);
310+
// when using quarto session cache, we ensure to cleanup the cache files at TempContext cleanup
311+
const cache = await sassCache(cacheDir, useSessionCache ? temp : undefined);
312312
return cache.getOrSet(input, loadPaths, temp, cacheIdentifier, compressed);
313313
} else {
314314
const outputFilePath = temp.createFile({ suffix: ".css" });

src/core/sass/cache.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,16 @@ class SassCache {
158158
}
159159

160160
// add a cleanup method to register a cleanup handler
161-
cleanup(removeCachePath: boolean = true) {
162-
onCleanup(() => {
161+
cleanup(temp: TempContext | undefined) {
162+
const registerCleanup = temp ? temp.onCleanup : onCleanup;
163+
registerCleanup(() => {
163164
try {
164165
this.kv.close();
165-
if (removeCachePath) safeRemoveIfExists(this.path);
166+
if (!temp) safeRemoveIfExists(this.path);
166167
} catch (error) {
167-
log.info("Error occurred during sass cache cleanup: " + error);
168+
log.info(
169+
`Error occurred during sass cache cleanup for ${this.path}: ${error}`,
170+
);
168171
}
169172
});
170173
}
@@ -205,9 +208,10 @@ async function checkVersion(kv: Deno.Kv, path: string) {
205208
}
206209

207210
const _sassCache: Record<string, SassCache> = {};
211+
208212
export async function sassCache(
209213
path: string,
210-
removeCachePath: boolean = false,
214+
temp: TempContext | undefined,
211215
): Promise<SassCache> {
212216
if (!_sassCache[path]) {
213217
log.debug(`Creating SassCache at ${path}`);
@@ -217,7 +221,7 @@ export async function sassCache(
217221
await checkVersion(kv, kvFile);
218222
_sassCache[path] = new SassCache(kv, path);
219223
// register cleanup for this cache
220-
_sassCache[path].cleanup(removeCachePath);
224+
_sassCache[path].cleanup(temp);
221225
}
222226
log.debug(`Returning SassCache at ${path}`);
223227
const result = _sassCache[path];

src/core/temp-types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ export interface TempContext {
99
createFile: (options?: Deno.MakeTempOptions) => string;
1010
createDir: (options?: Deno.MakeTempOptions) => string;
1111
cleanup: () => void;
12+
onCleanup: (handler: VoidFunction) => VoidFunction[];
1213
}

src/core/temp.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* Copyright (C) 2020-2022 Posit Software, PBC
55
*/
66

7-
import { debug } from "../deno_ral/log.ts";
7+
import { debug, info } from "../deno_ral/log.ts";
88
import { join } from "../deno_ral/path.ts";
99
import { ensureDirSync, existsSync } from "fs/mod.ts";
1010
import { normalizePath, removeIfExists, safeRemoveIfExists } from "./path.ts";
@@ -58,6 +58,9 @@ export function createTempContext(options?: Deno.MakeTempOptions) {
5858
...options,
5959
dir: tempDir,
6060
});
61+
62+
const tempContextCleanupHandlers: VoidFunction[] = [];
63+
6164
return {
6265
baseDir: dir,
6366
createFile: (options?: Deno.MakeTempOptions) => {
@@ -68,10 +71,22 @@ export function createTempContext(options?: Deno.MakeTempOptions) {
6871
},
6972
cleanup: () => {
7073
if (dir) {
74+
// Not using .reverse() to not mutate the original array
75+
for (let i = tempContextCleanupHandlers.length - 1; i >= 0; i--) {
76+
const handler = tempContextCleanupHandlers[i];
77+
try {
78+
handler();
79+
} catch (error) {
80+
info("Error occurred during tempContext handler cleanup: " + error);
81+
}
82+
}
7183
safeRemoveIfExists(dir);
7284
dir = undefined;
7385
}
7486
},
87+
onCleanup(handler: VoidFunction) {
88+
tempContextCleanupHandlers.push(handler);
89+
},
7590
};
7691
}
7792

0 commit comments

Comments
 (0)