refactor: clean up process management logic (#2205)

This should help fix various issues on Windows such as: - terminal window popping up when starting preview (& closing it kills the preview server) - process keeps running on Windows when VS Code is closed, preventing cleanup of resources - race condition bugs when using multiple VS Code windows on Mac
fwouts · Nov 29, 2023 · d5b9676 · d5b9676
1 parent babd8df
commit d5b9676
Show file tree

Hide file tree

Showing 18 changed files with 194 additions and 329 deletions.
diff --git a/core/src/index.ts b/core/src/index.ts
@@ -30,6 +30,7 @@ export type {
   FrameworkPluginFactory,
 } from "./plugins/framework.js";
 export type { OnServerStart } from "./preview-env.js";
+export { toVitePath } from "./vite/vite-paths.js";
 
 const require = createRequire(import.meta.url);
 

diff --git a/core/src/vite/plugins/virtual-plugin.ts b/core/src/vite/plugins/virtual-plugin.ts
@@ -4,6 +4,7 @@ import path from "path";
 import type { Logger } from "pino";
 import type * as vite from "vite";
 import { transformWithEsbuild } from "vite";
+import { toVitePath } from "../vite-paths.js";
 
 const VIRTUAL_PREFIX = `/@previewjs-virtual:`;
 const VIRTUAL_PREFIX2 = `/@id/__x00__`;
@@ -172,9 +173,7 @@ export function virtualPlugin(options: {
       if (!entry || entry.kind !== "file") {
         return;
       }
-      // Note: backslash handling is Windows-specific.
-      const virtualModuleId =
-        VIRTUAL_PREFIX + absoluteFilePath.replace(/\\/g, "/");
+      const virtualModuleId = VIRTUAL_PREFIX + toVitePath(absoluteFilePath);
       const node = moduleGraph.getModuleById(virtualModuleId);
       return node && [node];
     },

diff --git a/core/src/vite/vite-manager.ts b/core/src/vite/vite-manager.ts
@@ -35,6 +35,7 @@ import {
 } from "./plugins/preview-script.js";
 import { publicAssetImportPluginPlugin } from "./plugins/public-asset-import-plugin.js";
 import { virtualPlugin } from "./plugins/virtual-plugin.js";
+import { toVitePath } from "./vite-paths.js";
 
 const POSTCSS_CONFIG_FILE = [
   ".postcssrc",
@@ -513,7 +514,7 @@ export class ViteManager {
     const { viteServer, config } = state;
     if (info.virtual) {
       const modules = await viteServer.moduleGraph.getModulesByFile(
-        absoluteFilePath
+        toVitePath(absoluteFilePath)
       );
       for (const module of modules || []) {
         if (!module.id) {

diff --git a/core/src/vite/vite-paths.ts b/core/src/vite/vite-paths.ts
@@ -0,0 +1,9 @@
+export function toVitePath(absoluteFilePath: string) {
+  if (absoluteFilePath.match(/^[a-z]:/)) {
+    // Vite uses uppercase drive letters on Windows.
+    absoluteFilePath =
+      absoluteFilePath[0]?.toUpperCase() + absoluteFilePath.substring(1);
+  }
+  // Vite uses forward slash even on Windows.
+  return absoluteFilePath.replace(/\\/g, "/");
+}
diff --git a/daemon/package.json b/daemon/package.json
@@ -42,8 +42,6 @@
     "@previewjs/loader": "workspace:*",
     "@types/node": "^20.10.0",
     "exclusive-promises": "^1.0.3",
-    "exit-hook": "^3.2.0",
-    "is-wsl": "^3.1.0",
     "rimraf": "^5.0.5",
     "ts-node": "^10.9.1",
     "typescript": "^5.2.2",

diff --git a/daemon/src/api.ts b/daemon/src/api.ts
@@ -1,9 +1,3 @@
-export type KillRequest = Record<never, never>;
-
-export type KillResponse = {
-  pid: number;
-};
-
 export type CrawlFileRequest = {
   absoluteFilePath: string;
 };

diff --git a/daemon/src/client.ts b/daemon/src/client.ts
@@ -1,13 +1,10 @@
 import { exclusivePromiseRunner } from "exclusive-promises";
-import { existsSync, readFileSync, unlinkSync } from "fs";
 import http from "http";
 import type {
   CheckPreviewStatusRequest,
   CheckPreviewStatusResponse,
   CrawlFileRequest,
   CrawlFileResponse,
-  KillRequest,
-  KillResponse,
   StartPreviewRequest,
   StartPreviewResponse,
   StopPreviewRequest,
@@ -71,7 +68,6 @@ export function createClient(baseUrl: string): Client {
   }
 
   const client: Client = {
-    kill: () => makeRPC<KillRequest, KillResponse>("/previewjs/kill")({}),
     crawlFile: makeRPC("/crawl-file"),
     startPreview: makeRPC("/previews/start"),
     checkPreviewStatus: makeRPC("/previews/status"),
@@ -81,20 +77,7 @@ export function createClient(baseUrl: string): Client {
   return client;
 }
 
-export function destroyDaemon(lockFilePath: string) {
-  if (existsSync(lockFilePath)) {
-    const pid = parseInt(readFileSync(lockFilePath, "utf8"));
-    try {
-      process.kill(pid, "SIGKILL");
-    } catch {
-      // The daemon was already dead.
-    }
-    unlinkSync(lockFilePath);
-  }
-}
-
 export interface Client {
-  kill(): Promise<KillResponse>;
   crawlFile(request: CrawlFileRequest): Promise<CrawlFileResponse>;
   startPreview(request: StartPreviewRequest): Promise<StartPreviewResponse>;
   checkPreviewStatus(

diff --git a/daemon/src/index.ts b/daemon/src/index.ts
@@ -1,75 +1,20 @@
 import type { PreviewServer, Workspace } from "@previewjs/core";
 import { load } from "@previewjs/loader/runner";
-import exitHook from "exit-hook";
-import {
-  appendFileSync,
-  existsSync,
-  readFileSync,
-  unlinkSync,
-  writeFileSync,
-} from "fs";
+import { appendFileSync, writeFileSync } from "fs";
 import http from "http";
-import isWsl from "is-wsl";
 import path from "path";
 import type {
   CheckPreviewStatusRequest,
   CheckPreviewStatusResponse,
   CrawlFileRequest,
   CrawlFileResponse,
-  KillRequest,
-  KillResponse,
   StartPreviewRequest,
   StartPreviewResponse,
   StopPreviewRequest,
   StopPreviewResponse,
   UpdatePendingFileRequest,
   UpdatePendingFileResponse,
 } from "./api.js";
-import { createClient } from "./client.js";
-
-const lockFilePath = process.env.PREVIEWJS_LOCK_FILE;
-if (lockFilePath) {
-  if (existsSync(lockFilePath)) {
-    const pid = parseInt(readFileSync(lockFilePath, "utf8"));
-    try {
-      // Test if PID is still running. This will fail if not.
-      process.kill(pid, 0);
-    } catch {
-      // Previous process ended prematurely (e.g. hardware crash).
-      try {
-        unlinkSync(lockFilePath);
-      } catch {
-        // It's possible for several processes to try unlinking at the same time.
-        // For example, a running daemon that is exiting at the same time.
-        // Ignore.
-      }
-    }
-  }
-  try {
-    writeFileSync(lockFilePath, process.pid.toString(10), {
-      flag: "wx",
-    });
-    exitHook((signal) => {
-      // Note: The bracketed tag is required for VS Code and IntelliJ to detect exit.
-      process.stdout.write(
-        `[exit] Preview.js daemon shutting down with signal: ${signal}\n`
-      );
-      try {
-        unlinkSync(lockFilePath);
-      } catch {
-        // It's possible for several processes to try unlinking at the same time.
-        // For example, a new daemon that will replace this one.
-        // Ignore.
-      }
-    });
-  } catch {
-    // eslint-disable-next-line no-console
-    console.error(
-      `Unable to obtain lock: ${lockFilePath}\nYou can delete this file manually if you wish to override the lock.`
-    );
-    process.exit(1);
-  }
-}
 
 const logFilePath = process.env.PREVIEWJS_LOG_FILE;
 
@@ -112,59 +57,37 @@ export async function startDaemon({
   versionCode,
   port,
 }: DaemonStartOptions) {
+  const parentProcessId = parseInt(
+    process.env["PREVIEWJS_PARENT_PROCESS_PID"] || "0"
+  );
+  if (!parentProcessId) {
+    throw new Error(
+      "Missing environment variable: PREVIEWJS_PARENT_PROCESS_PID"
+    );
+  }
+
+  // Kill the daemon if the parent process dies.
+  setInterval(() => {
+    try {
+      process.kill(parentProcessId, 0);
+      // Parent process is still alive, see https://stackoverflow.com/a/21296291.
+    } catch (e) {
+      process.stdout.write(
+        `[exit] Parent process with PID ${parentProcessId} exited. Daemon exiting.\n`
+      );
+      process.exit(0);
+    }
+  }, 1000);
+
   const previewjs = await load({
-    installDir: loaderInstallDir,
+    installDir: loaderInstallDir.replace(/\//g, path.sep),
     workerFilePath: loaderWorkerPath,
     onServerStartModuleName,
   });
   const logger = previewjs.logger;
 
   const previewServers: Record<string, PreviewServer> = {};
   const endpoints: Record<string, (req: any) => Promise<any>> = {};
-  let wslRoot: string | null = null;
-
-  function transformAbsoluteFilePath(absoluteFilePath: string) {
-    if (!isWsl) {
-      return absoluteFilePath;
-    }
-    if (absoluteFilePath.match(/^[a-z]:.*$/i)) {
-      if (!wslRoot) {
-        wslRoot = detectWslRoot();
-      }
-      // This is a Windows path, which needs to be converted to Linux format inside WSL.
-      return `${wslRoot}/${absoluteFilePath
-        .substring(0, 1)
-        .toLowerCase()}/${absoluteFilePath.substring(3).replace(/\\/g, "/")}`;
-    }
-    // This is already a Linux path.
-    return absoluteFilePath;
-  }
-
-  function detectWslRoot() {
-    const wslConfPath = "/etc/wsl.conf";
-    const defaultRoot = "/mnt";
-    try {
-      if (!existsSync(wslConfPath)) {
-        return defaultRoot;
-      }
-      const configText = readFileSync(wslConfPath, "utf8");
-      const match = configText.match(/root\s*=\s*(.*)/);
-      if (!match) {
-        return defaultRoot;
-      }
-      const detectedRoot = match[1]!.trim();
-      if (detectedRoot.endsWith("/")) {
-        return detectedRoot.substring(0, detectedRoot.length - 1);
-      } else {
-        return detectedRoot;
-      }
-    } catch (e) {
-      logger.warn(
-        `Unable to read WSL config, assuming default root: ${defaultRoot}`
-      );
-      return defaultRoot;
-    }
-  }
 
   const app = http.createServer((req, res) => {
     if (req.headers["origin"]) {
@@ -236,23 +159,13 @@ export async function startDaemon({
 
   class NotFoundError extends Error {}
 
-  endpoint<KillRequest, KillResponse>("/previewjs/kill", async () => {
-    setTimeout(() => {
-      logger.info("Seppuku was requested. Bye bye.");
-      process.exit(0);
-    }, 1000);
-    return {
-      pid: process.pid,
-    };
-  });
-
   const inWorkspace = <T>(
     absoluteFilePath: string,
     run: (workspace: Workspace | null) => Promise<T>
   ) =>
     previewjs.inWorkspace({
       versionCode,
-      absoluteFilePath: transformAbsoluteFilePath(absoluteFilePath),
+      absoluteFilePath,
       run,
     });
 
@@ -265,10 +178,7 @@ export async function startDaemon({
         }
         const { components, stories } = await workspace.crawlFiles([
           path
-            .relative(
-              workspace.rootDir,
-              transformAbsoluteFilePath(absoluteFilePath)
-            )
+            .relative(workspace.rootDir, absoluteFilePath)
             .replace(/\\/g, "/"),
         ]);
         return {
@@ -326,47 +236,14 @@ export async function startDaemon({
   endpoint<UpdatePendingFileRequest, UpdatePendingFileResponse>(
     "/pending-files/update",
     async (req) => {
-      await previewjs.updateFileInMemory(
-        transformAbsoluteFilePath(req.absoluteFilePath),
-        req.utf8Content
-      );
+      await previewjs.updateFileInMemory(req.absoluteFilePath, req.utf8Content);
       return {};
     }
   );
 
   await new Promise<void>((resolve, reject) => {
     app.listen(port, resolve).on("error", async (e: any) => {
-      if (e.code !== "EADDRINUSE") {
-        return reject(e);
-      }
-      try {
-        // There's another daemon running already on the same port.
-        // Attempt to kill it and try again. This can happen for example
-        // when upgrading from one version to another of Preview.js.
-        const client = createClient(`http://localhost:${port}`);
-        const { pid } = await client.kill();
-        // Wait for daemon to be killed.
-        let oldDaemonDead = false;
-        for (let i = 0; !oldDaemonDead && i < 10; i++) {
-          try {
-            // Test if PID is still running. This will fail if not.
-            process.kill(pid, 0);
-            await new Promise((resolve) => setTimeout(resolve, 1000));
-          } catch {
-            oldDaemonDead = true;
-            app.listen(port, resolve).on("error", reject);
-          }
-        }
-        if (!oldDaemonDead) {
-          reject(
-            new Error(
-              `Unable to kill old daemon server running on port ${port}`
-            )
-          );
-        }
-      } catch (e) {
-        reject(e);
-      }
+      reject(e);
     });
   });
 

diff --git a/integrations/intellij/build.gradle.kts b/integrations/intellij/build.gradle.kts
@@ -69,7 +69,7 @@ tasks {
     }
     withType<KotlinCompile> {
         kotlinOptions.jvmTarget = "17"
-        kotlinOptions.freeCompilerArgs = listOf("-Xjvm-default=enable", "-Xopt-in=kotlin.RequiresOptIn")
+        kotlinOptions.freeCompilerArgs = listOf("-Xjvm-default=all", "-Xopt-in=kotlin.RequiresOptIn")
     }
 
     withType<Detekt> {
@@ -82,7 +82,15 @@ tasks {
             files(layout.buildDirectory)
         )
         exec {
-            commandLine("sh", "-c", "../../node_modules/turbo/bin/turbo run build --scope=@previewjs/intellij-daemon")
+            if (System.getProperty("os.name").lowercase().contains("win")) {
+                commandLine(
+                    "cmd.exe",
+                    "/c",
+                    "node " + System.getProperty("user.dir") + "\\..\\..\\node_modules\\turbo\\bin\\turbo run build --scope=@previewjs/intellij-daemon"
+                )
+            } else {
+                commandLine("sh", "-c", "../../node_modules/turbo/bin/turbo run build --scope=@previewjs/intellij-daemon")
+            }
         }
         from(daemonDir) {
             into("${properties("pluginName")}/daemon")