chore: close fd in js impl (#6760)

## What's the problem this PR addresses?

- We forgot the close the file descriptor when releasing JS files. This
causes EMFILE on some jobs.
- The cache was mistakenly removed from the libzip backend, causing
install times to jump, especially on pnpm installs.

Fixes #6758

## Checklist

<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.

---------

Co-authored-by: Maël Nison <[email protected]>

Author: goloveychuk

.pnp.cjs

@@ -0,0 +1,31 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/libzip": patch
+  "@yarnpkg/pnp": patch
+
+declined:
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-file"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - vscode-zipfs
+  - "@yarnpkg/builder"
+  - "@yarnpkg/core"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/fslib"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"

.yarn/versions/baa73f5a.yml

@@ -76,6 +76,7 @@ export interface Stat {
 }
 
 export interface ZipImpl {
+  filesShouldBeCached: boolean;
   deleteEntry(index: number): void;
   getFileSource(index: number): {data: Buffer, compressionMethod: number};
   setFileSource(target: PortablePath, compression: CompressionData, buffer: Buffer): number;
@@ -109,6 +110,12 @@ export class ZipFS extends BasePortableFakeFS {
   private readonly listings: Map<PortablePath, Set<Filename>> = new Map();
   private readonly entries: Map<PortablePath, number> = new Map();
 
+  /**
+   * A cache of indices mapped to file sources.
+   * Populated by `setFileSource` calls.
+   * Required for supporting read after write.
+   */
+  private readonly fileSources: Map<number, Buffer> = new Map();
 
   private symlinkCount: number;
 
@@ -736,6 +743,8 @@ export class ZipFS extends BasePortableFakeFS {
     if (typeof entry === `undefined`)
       return;
 
+    this.fileSources.delete(entry);
+
     if (this.isSymbolicLink(entry)) {
       this.symlinkCount--;
     }
@@ -819,6 +828,7 @@ export class ZipFS extends BasePortableFakeFS {
     }
 
     const newIndex = this.zipImpl.setFileSource(target, compression, buffer);
+    this.fileSources.set(newIndex, buffer);
 
     return newIndex;
   }
@@ -842,8 +852,15 @@ export class ZipFS extends BasePortableFakeFS {
   private getFileSource(index: number, opts: {asyncDecompress: true}): Promise<Buffer>;
   private getFileSource(index: number, opts: {asyncDecompress: boolean}): Promise<Buffer> | Buffer;
   private getFileSource(index: number, opts: {asyncDecompress: boolean} = {asyncDecompress: false}): Promise<Buffer> | Buffer {
+    const cachedFileSource = this.fileSources.get(index);
+    if (typeof cachedFileSource !== `undefined`)
+      return cachedFileSource;
+
     const {data, compressionMethod} = this.zipImpl.getFileSource(index);
     if (compressionMethod === STORE) {
+      if (this.zipImpl.filesShouldBeCached)
+        this.fileSources.set(index, data);
+
       return data;
     } else if (compressionMethod === DEFLATE) {
       if (opts.asyncDecompress) {
@@ -852,12 +869,17 @@ export class ZipFS extends BasePortableFakeFS {
             if (error) {
               reject(error);
             } else {
+              if (this.zipImpl.filesShouldBeCached)
+                this.fileSources.set(index, result);
               resolve(result);
             }
           });
         });
       } else {
-        return zlib.inflateRawSync(data);
+        const decompressedData = zlib.inflateRawSync(data);
+        if (this.zipImpl.filesShouldBeCached)
+          this.fileSources.set(index, decompressedData);
+        return decompressedData;
       }
     } else {
       throw new Error(`Unsupported compression method: ${compressionMethod}`);

packages/yarnpkg-libzip/sources/ZipFS.ts

@@ -23,9 +23,10 @@ export interface Entry {
 }
 
 export class JsZipImpl implements ZipImpl {
-  fd: number;
-  baseFs: FakeFS<PortablePath>;
-  entries: Array<Entry>;
+  private fd: number | `closed`;
+  private baseFs: FakeFS<PortablePath>;
+  private entries: Array<Entry>;
+  public filesShouldBeCached = false;
 
   constructor(opts: ZipImplInput) {
     if (`buffer` in opts)
@@ -37,7 +38,13 @@ export class JsZipImpl implements ZipImpl {
     this.baseFs = opts.baseFs;
     this.fd = this.baseFs.openSync(opts.path, `r`);
 
-    this.entries = JsZipImpl.readZipSync(this.fd, this.baseFs, opts.size);
+    try {
+      this.entries = JsZipImpl.readZipSync(this.fd, this.baseFs, opts.size);
+    } catch (error) {
+      this.baseFs.closeSync(this.fd);
+      this.fd = `closed`;
+      throw error;
+    }
   }
 
   static readZipSync(fd: number, baseFs: FakeFS<PortablePath>, fileSize: number): Array<Entry> {
@@ -213,7 +220,10 @@ export class JsZipImpl implements ZipImpl {
     return -1;
   }
 
-  getFileSource(index: number): {data: Buffer, compressionMethod: number} {
+  getFileSource(index: number) {
+    if (this.fd === `closed`)
+      throw new Error(`ZIP file is closed`);
+
     const entry = this.entries[index];
     const localHeaderBuf = Buffer.alloc(30);
 
@@ -237,6 +247,10 @@ export class JsZipImpl implements ZipImpl {
   }
 
   discard(): void {
+    if (this.fd !== `closed`) {
+      this.baseFs.closeSync(this.fd);
+      this.fd = `closed`;
+    }
   }
 
   addDirectory(path: string): number {

packages/yarnpkg-libzip/sources/jsZipImpl.ts

@@ -20,17 +20,11 @@ export class LibZipImpl implements ZipImpl {
   private readonly libzip: Libzip;
   private readonly lzSource: number;
   private readonly zip: number;
-
-  /**
-   * A cache of indices mapped to file sources.
-   * Populated by `setFileSource` calls.
-   * Required for supporting read after write.
-   */
-  private readonly fileSources: Map<number, Buffer> = new Map();
-
   private readonly listings: Array<string>;
   private readonly symlinkCount: number;
 
+  public filesShouldBeCached = true;
+
   constructor(opts: ZipImplInput) {
     const buffer = `buffer` in opts
       ? opts.buffer
@@ -127,7 +121,6 @@ export class LibZipImpl implements ZipImpl {
           throw this.makeLibzipError(this.libzip.getError(this.zip));
         }
       }
-      this.fileSources.set(newIndex, buffer);
       return newIndex;
     } catch (error) {
       this.libzip.source.free(lzSource);
@@ -164,10 +157,6 @@ export class LibZipImpl implements ZipImpl {
   }
 
   getFileSource(index: number) {
-    const cachedFileSource = this.fileSources.get(index);
-    if (typeof cachedFileSource !== `undefined`)
-      return {data: cachedFileSource, compressionMethod: 0};
-
     const stat = this.libzip.struct.statS();
 
     const rc = this.libzip.statIndex(this.zip, index, 0, 0, stat);
@@ -206,7 +195,6 @@ export class LibZipImpl implements ZipImpl {
   }
 
   deleteEntry(index: number) {
-    this.fileSources.delete(index);
     const rc = this.libzip.delete(this.zip, index);
     if (rc === -1) {
       throw this.makeLibzipError(this.libzip.getError(this.zip));