fix(kernel): EPERM on rename operation on Windows (#3769)

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



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

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';
@@ -113,10 +111,11 @@ export class Kernel {
     const originalUmask = process.umask(0o022);
     try {
       // 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 +127,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,49 @@ let packageCacheEnabled =
  */
 export function extract(
   file: string,
+  outDir: string,
   options: ExtractOptions,
   ...comments: readonly string[]
 ): ExtractResult {
-  return (packageCacheEnabled ? extractToCache : extractToTemporary)(
-    file,
-    options,
-    ...comments,
-  );
+  mkdirSync(outDir, { recursive: true });
+  try {
+    return (packageCacheEnabled ? extractViaCache : extractToOutDir)(
+      file,
+      outDir,
+      options,
+      ...comments,
+    );
+  } catch (err) {
+    rmSync(outDir, { force: true, recursive: true });
+    throw err;
+  }
 }
 
-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 +89,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(