From c98e7c1eb93359c0c7dfd9e750d017fdce0cbc34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=A7=91=F0=9F=8F=BB=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier?= Date: Mon, 26 Sep 2022 14:58:21 +0200 Subject: [PATCH] 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 | 19 ++----- packages/@jsii/kernel/src/tar-cache/index.ts | 55 +++++++++++--------- 2 files changed, 35 insertions(+), 39 deletions(-) diff --git a/packages/@jsii/kernel/src/kernel.ts b/packages/@jsii/kernel/src/kernel.ts index 36d2ee571d..7b099cdf67 100644 --- a/packages/@jsii/kernel/src/kernel.ts +++ b/packages/@jsii/kernel/src/kernel.ts @@ -1,7 +1,6 @@ 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'; @@ -9,7 +8,6 @@ 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 diff --git a/packages/@jsii/kernel/src/tar-cache/index.ts b/packages/@jsii/kernel/src/tar-cache/index.ts index 5bb6d3d9bc..d30f07ce1e 100644 --- a/packages/@jsii/kernel/src/tar-cache/index.ts +++ b/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(