iden3 / snarkjs

zkSNARK implementation in JavaScript & WASM
GNU General Public License v3.0
1.78k stars 421 forks source link

Workers keep running longer than expected #152

Open Schaeff opened 2 years ago

Schaeff commented 2 years ago

While using mocha to test some snarkjs setup and proof generation, the test suite ends up hanging after all tests pass. I checked why that was the case using why-is-node-running in my after function and the result points to worker threads in snarkjs:

# WORKER
node:internal/async_hooks:201                                                        
node:internal/worker:185                                                             
/node_modules/web-worker/cjs/node.js:111       - const worker = new threads.Worker(__filename, {
/node_modules/ffjavascript/build/main.cjs:5244 - tm.workers[i] = new Worker__default["default"](workerSource);
/node_modules/ffjavascript/build/main.cjs:6538 - const tm = await buildThreadManager(params.wasm, params.singleThread);
/node_modules/ffjavascript/build/main.cjs:6635 - const curve = await buildEngine(params);
/node_modules/snarkjs/build/main.cjs:53        - curve = await ffjavascript.buildBn128();
/node_modules/snarkjs/build/main.cjs:1435      - const curve = await getCurveFromQ(q);
/node_modules/snarkjs/build/main.cjs:4082      - const {curve, power} = await readPTauHeader(fdPTau, sectionsPTau);

In the tests to this repo I found the following:

    before( async () => {
        curve = await getCurveFromName("bn128");
//        curve.Fr.s = 10;
    });
    after( async () => {
        await curve.terminate();
        // console.log(process._getActiveHandles());
        // console.log(process._getActiveRequests());
    });

If calling curve.terminate() is the solution to close the workers, is there a way to do the same thing when using, say, snarkjs.groth16.prove(zkeyPath, witnessPath)?

Apologies if I'm missing something obvious, my javascript is a bit rusty!

Cheers

phated commented 2 years ago

Which version of snarkjs is this happening on? I did work in more recent versions of ffjavascript that should have solved this.

Schaeff commented 2 years ago

Thanks for the quick response! package-lock.json has 0.4.19

phated commented 2 years ago

@Schaeff I'm still looking into this, but I did remember that ffjavascript attaches the curves to a global object. So if you are using bn128, you could do globalThis.curve_bn128.terminate()

I'm on the fence if snarkjs should terminate the curves itself, since some code might do multiple operations with the same curve and we don't want to re-initialize it. Maybe we could terminate them only in the CLI or maybe behind a flag. Thoughts?

Schaeff commented 2 years ago

This worked, thanks! I do not have a strong opinion, I just wanted the tests to end cleanly. Maybe documenting this detail better would be enough? This can be closed as far as I am concerned.

biscuitdey commented 1 year ago

Inside jest test suite, await snarkjs.groth16.fullProve( input, wasm, key); is causing the test run to hang. This problem is still persisting.

kyc_app_be/node_modules/ffjavascript/build/main.cjs:1291 - crypto__default["default"].randomFillSync(array);

kyc_app_be/node_modules/ffjavascript/build/main.cjs:1297 - const arr = getRandomBytes(32);

kyc_app_be/node_modules/ffjavascript/build/main.cjs:1310 - threadRng = new ChaCha(getRandomSeed());

kyc_app_be/node_modules/ffjavascript/build/main.cjs:3383 - return this.fromRng(getThreadRng());

kyc_app_be/node_modules/snarkjs/build/main.cjs:930 - const r = curve.Fr.random();

kyc_app_be/node_modules/snarkjs/build/main.cjs:1275 - return await groth16Prove(zkeyFileName, wtns, logger);

erhant commented 1 year ago

I also had this problem with Jest and I would like to note that --detectOpenHandles does not detect this problem. I was just --forceExit'ing until I saw the solution here. As a snippet, just add:

afterAll(async () => {
  // you can @ts-ignore if you are using ts-jest or such
  await globalThis.curve_bn128.terminate();
});
SnickerChar commented 7 months ago

Was experiencing this exact problem and started looking at the issues. I'm glad there's a fix available, and indeed, Jest's --detectOpenHandles does not detect this!