Merge pull request #10372 from quarto-dev/sass-cache-cleanup

Author: cderv

src/core/cleanup.ts

@@ -1,9 +1,8 @@
 /*
-* cleanup.ts
-*
-* Copyright (C) 2020-2022 Posit Software, PBC
-*
-*/
+ * cleanup.ts
+ *
+ * Copyright (C) 2020-2022 Posit Software, PBC
+ */
 
 import { info } from "../deno_ral/log.ts";
 
@@ -14,7 +13,9 @@ export function onCleanup(handler: VoidFunction) {
 }
 
 export function exitWithCleanup(code: number) {
-  for (const handler of cleanupHandlers) {
+  // Not using cleanupHandlers.reverse() to not mutate the original array
+  for (let i = cleanupHandlers.length - 1; i >= 0; i--) {
+    const handler = cleanupHandlers[i];
     try {
       handler();
     } catch (error) {

src/core/sass.ts

@@ -307,7 +307,8 @@ export async function compileWithCache(
     const cacheDir = useSessionCache
       ? join(temp.baseDir, "sass")
       : quartoCacheDir("sass");
-    const cache = await sassCache(cacheDir);
+    // when using quarto session cache, we ensure to cleanup the cache files at TempContext cleanup
+    const cache = await sassCache(cacheDir, useSessionCache ? temp : undefined);
     return cache.getOrSet(input, loadPaths, temp, cacheIdentifier, compressed);
   } else {
     const outputFilePath = temp.createFile({ suffix: ".css" });

src/core/sass/cache.ts

@@ -14,6 +14,7 @@ import { dartCompile } from "../dart-sass.ts";
 import { TempContext } from "../temp.ts";
 import { safeRemoveIfExists } from "../path.ts";
 import * as log from "../../deno_ral/log.ts";
+import { onCleanup } from "../cleanup.ts";
 
 class SassCache {
   kv: Deno.Kv;
@@ -155,6 +156,21 @@ class SassCache {
       compressed,
     );
   }
+
+  // add a cleanup method to register a cleanup handler
+  cleanup(temp: TempContext | undefined) {
+    const registerCleanup = temp ? temp.onCleanup : onCleanup;
+    registerCleanup(() => {
+      try {
+        this.kv.close();
+        if (temp) safeRemoveIfExists(this.path);
+      } catch (error) {
+        log.info(
+          `Error occurred during sass cache cleanup for ${this.path}: ${error}`,
+        );
+      }
+    });
+  }
 }
 
 const currentSassCacheVersion = 1;
@@ -192,14 +208,20 @@ async function checkVersion(kv: Deno.Kv, path: string) {
 }
 
 const _sassCache: Record<string, SassCache> = {};
-export async function sassCache(path: string): Promise<SassCache> {
+
+export async function sassCache(
+  path: string,
+  temp: TempContext | undefined,
+): Promise<SassCache> {
   if (!_sassCache[path]) {
     log.debug(`Creating SassCache at ${path}`);
     ensureDirSync(path);
     const kvFile = join(path, "sass.kv");
     const kv = await Deno.openKv(kvFile);
     await checkVersion(kv, kvFile);
     _sassCache[path] = new SassCache(kv, path);
+    // register cleanup for this cache
+    _sassCache[path].cleanup(temp);
   }
   log.debug(`Returning SassCache at ${path}`);
   const result = _sassCache[path];

src/core/temp-types.ts

@@ -9,4 +9,5 @@ export interface TempContext {
   createFile: (options?: Deno.MakeTempOptions) => string;
   createDir: (options?: Deno.MakeTempOptions) => string;
   cleanup: () => void;
+  onCleanup: (handler: VoidFunction) => void;
 }

src/core/temp.ts

@@ -4,7 +4,7 @@
  * Copyright (C) 2020-2022 Posit Software, PBC
  */
 
-import { debug } from "../deno_ral/log.ts";
+import { debug, info } from "../deno_ral/log.ts";
 import { join } from "../deno_ral/path.ts";
 import { ensureDirSync, existsSync } from "fs/mod.ts";
 import { normalizePath, removeIfExists, safeRemoveIfExists } from "./path.ts";
@@ -53,11 +53,14 @@ export function globalTempContext() {
   return tempContext;
 }
 
-export function createTempContext(options?: Deno.MakeTempOptions) {
+export function createTempContext(options?: Deno.MakeTempOptions): TempContext {
   let dir: string | undefined = Deno.makeTempDirSync({
     ...options,
     dir: tempDir,
   });
+
+  const tempContextCleanupHandlers: VoidFunction[] = [];
+
   return {
     baseDir: dir,
     createFile: (options?: Deno.MakeTempOptions) => {
@@ -68,10 +71,22 @@ export function createTempContext(options?: Deno.MakeTempOptions) {
     },
     cleanup: () => {
       if (dir) {
+        // Not using .reverse() to not mutate the original array
+        for (let i = tempContextCleanupHandlers.length - 1; i >= 0; i--) {
+          const handler = tempContextCleanupHandlers[i];
+          try {
+            handler();
+          } catch (error) {
+            info("Error occurred during tempContext handler cleanup: " + error);
+          }
+        }
         safeRemoveIfExists(dir);
         dir = undefined;
       }
     },
+    onCleanup(handler: VoidFunction) {
+      tempContextCleanupHandlers.push(handler);
+    },
   };
 }
 

src/project/serve/serve.ts

@@ -6,7 +6,13 @@
 
 import { info, warning } from "../../deno_ral/log.ts";
 import { existsSync } from "fs/mod.ts";
-import { basename, dirname, extname, join, relative } from "../../deno_ral/path.ts";
+import {
+  basename,
+  dirname,
+  extname,
+  join,
+  relative,
+} from "../../deno_ral/path.ts";
 import * as colors from "fmt/colors.ts";
 
 import * as ld from "../../core/lodash.ts";
@@ -906,7 +912,7 @@ function acquirePreviewLock(project: ProjectContext) {
   // write our pid to the lockfile
   Deno.writeTextFileSync(lockfile, String(Deno.pid));
 
-  // rmeove the lockfile when we exit
+  // remove the lockfile when we exit
   onCleanup(() => releasePreviewLock(project));
 }
 

tests/verify.ts

@@ -111,7 +111,7 @@ export const noErrorsOrWarnings: Verify = {
   name: "No Errors or Warnings",
   verify: (outputs: ExecuteOutput[]) => {
     const isErrorOrWarning = (output: ExecuteOutput) => {
-      return output.levelName.toLowerCase() === "warning" ||
+      return output.levelName.toLowerCase() === "warn" ||
         output.levelName.toLowerCase() === "error";
     };