zlib: avoid waiting for paused ZIP iterators

Project: nodejs/node | Language: JavaScript | Concepts: concurrency, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/nodejs/node/pull/65278

The problem

When reading ZIP entries using an async iterator, if the iterator is paused (i.e., after yielding a chunk but before the next call), calling ZipFile.close() would hang indefinitely. This is because the system counted the paused iterator as an "active read," so close() would wait forever, even though no I/O was happening.

Why it's non-trivial

A naive fix might simply ignore all iterators or always count them as active, but this would risk either closing resources while reads are still in progress (causing data loss or corruption) or never closing at all (causing resource leaks or hangs). The solution needs to distinguish between actual in-flight I/O and an iterator that's merely paused between chunks.

The approach

The patch refines how "active reads" are tracked. Instead of counting the whole iterator as a single read, it increments the count only during actual I/O, and decrements it immediately after yielding a chunk.

Key changes in the iterator:

-    // Count the whole stream as one in-flight read so a concurrent close()
-    // waits for it (the finally runs when the consumer stops iterating); a
-    // stream begun after close() was requested is rejected up front.
+    // Count active reads, but not time paused at a yield.

Annotation: The comment is updated to clarify that only active reads (not paused iterators) are counted.

-    handle.reads++;
+    let reading = true;

Annotation: Instead of incrementing the active read count for the entire iterator, a reading flag is introduced to track when I/O is actually happening.

@@ -521,10 +520,18 @@ class ZipEntry {
         await readFdFully(this.#liveDescriptor(), chunk, pos);
         pos += take;
         remaining -= take;
+        reading = false;
+        endHandleRead(handle);
         yield chunk;
+        if (handle.closing) {
+          throw new ERR_INVALID_STATE(
+            'cannot read a ZipEntry after its backing ZipFile has been closed');
+        }
+        handle.reads++;
+        reading = true;

Annotation:

• After each chunk is read, reading is set to false and endHandleRead(handle) is called, decrementing the active read count before yielding the chunk.
• After yielding, if the file is being closed, an error is thrown.
• Otherwise, the read count is incremented again and reading is set back to true before the next I/O.

@@ -521,10 +520,18 @@ class ZipEntry {
     } finally {
-      endHandleRead(handle);
+      if (reading) endHandleRead(handle);
     }

Annotation: In the finally block, endHandleRead(handle) is only called if a read is actually in progress, preventing double-decrementing.

Test case added:

+test('close() resolves with a paused contentIterator()', async () => {
+  tmpdir.refresh();
+  const file = tmpdir.resolve('paused-iterator.zip');
+  const payload = Buffer.alloc(8 * 1024 * 1024, 0x61);
+  writeArchive(file, [zlib.ZipEntry.createSync('big', payload, { method: 'store' })]);
+
+  const zf = zlib.ZipFile.openSync(file);
+  const iterator = zf.getSync('big').contentIterator();
+  const first = await iterator.next();
+  assert.strictEqual(first.done, false);
+
+  await zf.close();
+  await assert.rejects(iterator.next(), { code: 'ERR_INVALID_STATE' });
+});

Annotation:

• This test pauses the iterator after the first chunk, calls close(), and then asserts that further iteration fails with the correct error. This verifies that close() no longer hangs on a paused iterator.

From the review thread

Copilot:

This regression test can hang indefinitely on failure (the pre-fix behavior is that zf.close() never settles while the iterator is paused), which can stall CI until a global timeout. Add a per-test timeout (or an explicit Promise.race watchdog) so the failure mode is a fast, actionable assertion instead of a hang.

Takeaways

• When managing concurrent I/O, track only actual in-flight operations, not paused or idle states, to avoid resource leaks and deadlocks.
• Always update or add regression tests to cover edge cases, especially those that could cause hangs or indefinite waits.
• In async code, ensure that resource cleanup logic (like finally blocks) accurately reflects the current state to prevent double-free or missed-release bugs.