fflate and Node v22

[HowardBraham]

How to reproduce

The fflate library appears to be incompatible with Node v22, now the LTS version.

It fails and hangs in our application.

The problem

Running the test suite in Node v22 produces this result:

$ TS_NODE_PROJECT=test/tsconfig.json uvu -b -r ts-node/register test
0-valid.ts
 compression  ✘ • • • •   (4 / 5)

   FAIL  compression  "basic"
    Cannot transfer object of unsupported type.

    at Object.deflate (/workspaces/fflate/test/util.ts:122:20)
    at Object.<anonymous> (/workspaces/fflate/test/0-valid.ts:10:34)
    at step (/workspaces/fflate/test/0-valid.ts:33:23)
    at Object.next (/workspaces/fflate/test/0-valid.ts:14:53)
    at /workspaces/fflate/test/0-valid.ts:8:71
    at new Promise (<anonymous>)
    at __awaiter (/workspaces/fflate/test/0-valid.ts:4:12)
    at Object.compression (/workspaces/fflate/test/0-valid.ts:46:16)


error Command failed with exit code 1.

[101arrowz]

That's a bit concerning - I'll have a look tomorrow.

[101arrowz]

OK, this is just a bug in the test-bench because I was trying to transfer a Buffer with allocUnsafe (which can return a reference to a non-transferable buffer pool), instead of allocUnsafeSlow (which avoids using the the pool at all).

Is there a larger issue you're running into with Node v22 that causes the hanging behavior you've described?

[davidmurdoch]

We use this in the MetaMask Extension's build system and discovered it when attempting to update to Node v22 (our issue: MetaMask/metamask-extension#28368)

We use it here: https://github.com/MetaMask/metamask-extension/blob/39528b02100a6003f412bbef5e0b560002c945bc/development/webpack/utils/plugins/ManifestPlugin/index.ts#L48-L63 (ignore the "use a copy of the Buffer" comment, the Buffer copy operation is done elsewhere)

[HowardBraham]

I'm not sure if it helps, but if you add this test to your suite, it passes on Node 20 but fails on Node 22. It also passes if you use ZipDeflate instead of AsyncZipDeflate.

import { testSuites } from "./util";
import { AsyncZipDeflate, Zip } from '../src/index';

testSuites({
  async asyncZipDeflate(file) {
    const zip = new Zip((err, dat, final) => {
      if (!err) {
        console.log(dat, final);
      }
    });

    const helloTxt = new AsyncZipDeflate('hello.txt', {
      level: 0
    });

    zip.add(helloTxt);
    helloTxt.push(file, true);
    zip.end();
  }
});

[HowardBraham]

@101arrowz any ideas how to solve this?

[pedrokehl]

@davidmurdoch @HowardBraham
I was checking this out, given I'm about to add fflate as dependency to one of my projects, and we definitely want support to v22, and I noticed that the test you have is wrong, you are passing a Buffer instead of Uint8Array to helloTxt.push(), try converting it to Uint8Array and see how that goes, same on the metamask-extension codebase! You can do that with fflate.strToU8(file.toString()) 🙂

BTW, the change of allocUnsafe to allocUnsafeSlow is still needed, but that's just some "test suite" helper of the package.

[davidmurdoch]

In Node.js instances of Buffer are also instances of UInt8Array: https://nodejs.org/docs/latest-v22.x/api/buffer.html#buffer

The Buffer class is a subclass of JavaScript's Uint8Array class and extends it with methods that cover additional use cases.

[101arrowz]

The real issue here is with the tests that Node.js has decided to disable the old behavior of silently allowing attempted transfers of buffers that are views into the global Buffer pool. Transferring those buffers was never possible (otherwise all pooled buffers would be invalidated - not good) but fflate assumed it was fine to attempt to transfer them because I thought that the Node runtime would just copy the memory on an attempted transfer of a pooled Buffer. I thought this was a reasonable assumption since pooling is an implementation detail of Node.js, and IMO it really is the responsibility of Node to deal with the consequences of that decision.

My assumption was accurate until Node.js version 22; now however, Node throws an error when you attempt to transfer a pooled Buffer object. That's what the linked issue discusses.

This issue does not affect any synchronous functions because no data needs to be transferred between threads. It also doesn't affect most of the asynchronous functions because they do not assume consume the input by default (i.e. they copy memory to the other thread instead of transferring). However, it does affect the streaming asynchronous APIs, including AsyncZipDeflate and cousins, because those do consume the input (transfer memory instead of copying) by default.

What I'm planning to do is special-case the Node.js worker implementation to skip transferring buffers that are pooled altogether, which will essentially restore the old functionality. For now, you can work around the issue by copying all buffers before passing them into a streaming asynchronous API:

const myZipDeflate = new fflate.AsyncZipDeflate(...);.

const data = fs.readFileSync('hello.txt');

// instead of this:
// myZipDeflate.push(data)

// do this:
myZipDeflate.push(new Uint8Array(data));

// or this, if you want to save some copies:
const { isMarkedAsUntransferable } = require('worker_threads');

let transferableBuffer = isMarkedAsUntransferable(data) ? new Uint8Array(data) : data;
myZipDeflate.push(transferableBuffer);

(BTW: the reason "converting" a Buffer to Uint8Array avoids the issue is that by doing so you reallocate the memory into a non-pooled allocation that can be safely transferred.)