fix(kernel): EPERM on rename operation on Windows

Windows does not allow renaming files that are currently open, or
directories that contain open files. When antivirus software is used,
files are open for analysis by the A/V software, making it impossible to
rename files too quickly after they've been created.

This was already reported in #992 and addressed, however the issue was
re-introduced in #3724, in which tarballs were extracted into temporary
directories that were then renamed.

Changed the code back to a form taht extracts files directly into their
final place instead of staging via a temporary space, and added comments
warning maintainers about the specific issue being solved here, so that
hopefully the problem does not get re-introduced again in the future.

Fixes #3751

packages/@jsii/kernel/src/kernel.ts

@@ -1,15 +1,13 @@
 import * as spec from '@jsii/spec';
 import { loadAssemblyFromPath } from '@jsii/spec';
 import * as cp from 'child_process';
-import { renameSync } from 'fs';
 import * as fs from 'fs-extra';
 import { createRequire } from 'module';
 import * as os from 'os';
 import * as path from 'path';
 
 import * as api from './api';
 import { TOKEN_REF } from './api';
-import { link } from './link';
 import { jsiiTypeFqn, ObjectTable, tagJsiiConstructor } from './objects';
 import * as onExit from './on-exit';
 import * as wire from './serialization';
@@ -112,11 +110,15 @@ export class Kernel {
     // Force umask to have npm-install-like permissions
     const originalUmask = process.umask(0o022);
     try {
+      // Create the install directory (there may be several path components for @scoped/packages)
+      fs.mkdirSync(path.dirname(packageDir), { recursive: true });
+
       // untar the archive to its final location
-      const { path: extractedTo, cache } = this._debugTime(
+      const { cache } = this._debugTime(
         () =>
           tar.extract(
             req.tarball,
+            packageDir,
             {
               strict: true,
               strip: 1, // Removes the 'package/' path element from entries
@@ -128,21 +130,10 @@ export class Kernel {
         `tar.extract(${req.tarball}) => ${packageDir}`,
       );
 
-      // Create the install directory (there may be several path components for @scoped/packages)
-      fs.mkdirSync(path.dirname(packageDir), { recursive: true });
       if (cache != null) {
         this._debug(
           `Package cache enabled, extraction resulted in a cache ${cache}`,
         );
-
-        // Link the package into place.
-        this._debugTime(
-          () => link(extractedTo, packageDir),
-          `link(${extractedTo}, ${packageDir})`,
-        );
-      } else {
-        // This is not from cache, so we move it around instead of copying.
-        renameSync(extractedTo, packageDir);
       }
     } finally {
       // Reset umask to the initial value

packages/@jsii/kernel/src/tar-cache/index.ts

@@ -1,9 +1,8 @@
-import { mkdirSync, mkdtempSync, renameSync, rmSync } from 'fs';
-import { tmpdir } from 'os';
-import { join } from 'path';
+import { mkdirSync, rmSync } from 'fs';
 import * as tar from 'tar';
 
 import { DiskCache } from '../disk-cache';
+import { link } from '../link';
 import { defaultCacheRoot } from './default-cache-root';
 
 export type ExtractOptions = Omit<
@@ -12,11 +11,6 @@ export type ExtractOptions = Omit<
 >;
 
 export interface ExtractResult {
-  /**
-   * The path in which the extracted files are located
-   */
-  readonly path: string;
-
   /**
    * When `'hit'`, the data was already present in cache and was returned from
    * cache.
@@ -43,38 +37,43 @@ let packageCacheEnabled =
  */
 export function extract(
   file: string,
+  outDir: string,
   options: ExtractOptions,
   ...comments: readonly string[]
 ): ExtractResult {
-  return (packageCacheEnabled ? extractToCache : extractToTemporary)(
+  return (packageCacheEnabled ? extractViaCache : extractToOutDir)(
     file,
+    outDir,
     options,
     ...comments,
   );
 }
 
-function extractToCache(
+function extractViaCache(
   file: string,
+  outDir: string,
   options: ExtractOptions = {},
   ...comments: readonly string[]
-): { path: string; cache: 'hit' | 'miss' } {
+): { cache: 'hit' | 'miss' } {
   const cacheRoot =
     process.env.JSII_RUNTIME_PACKAGE_CACHE_ROOT ?? defaultCacheRoot();
-  const cache = DiskCache.inDirectory(cacheRoot);
+  const dirCache = DiskCache.inDirectory(cacheRoot);
 
-  const entry = cache.entryFor(file, ...comments);
-  return entry.lock((lock) => {
+  const entry = dirCache.entryFor(file, ...comments);
+  const { path, cache } = entry.lock((lock) => {
     let cache: 'hit' | 'miss' = 'hit';
     if (!entry.pathExists) {
-      const tmpPath = `${entry.path}.tmp`;
-      mkdirSync(tmpPath, { recursive: true });
+      // !!!IMPORTANT!!!
+      // Extract directly into the final target directory, as certain antivirus
+      // software configurations on Windows will make a `renameSync` operation
+      // fail with EPERM until the files have been fully analyzed.
+      mkdirSync(entry.path, { recursive: true });
       try {
         untarInto({
           ...options,
-          cwd: tmpPath,
+          cwd: entry.path,
           file,
         });
-        renameSync(tmpPath, entry.path);
       } catch (error) {
         rmSync(entry.path, { force: true, recursive: true });
         throw error;
@@ -84,17 +83,23 @@ function extractToCache(
     lock.touch();
     return { path: entry.path, cache };
   });
+
+  link(path, outDir);
+
+  return { cache };
 }
 
-function extractToTemporary(
+function extractToOutDir(
   file: string,
+  cwd: string,
   options: ExtractOptions = {},
-): { path: string } {
-  const path = mkdtempSync(join(tmpdir(), 'jsii-runtime-untar-'));
-
-  untarInto({ ...options, cwd: path, file });
-
-  return { path };
+): { cache?: undefined } {
+  // !!!IMPORTANT!!!
+  // Extract directly into the final target directory, as certain antivirus
+  // software configurations on Windows will make a `renameSync` operation
+  // fail with EPERM until the files have been fully analyzed.
+  untarInto({ ...options, cwd, file });
+  return {};
 }
 
 function untarInto(