Firaenix / bsv-wasm

BSV stdlib written in Rust and runs in WASM environments
MIT License
70 stars 19 forks source link

Memory leak when generating private keys #44

Closed breavyn closed 2 years ago

breavyn commented 2 years ago

Tested with Node v16.10.0 on Linux.

If we take a simple example

const bsv = require("bsv-wasm");
while (true) {
  const key = bsv.PrivateKey.fromRandom();
}

and run it, after 5 minutes we've nearly hit the heap limit

  PID  RES    SHR  %CPU  %MEM     TIME+ COMMAND
 7526 3.7g  21908 138.5   5.9   5:06.24 node

and kaboom!

wasm://wasm/002542aa:1

RuntimeError: unreachable
    at <anonymous>:wasm-function[928]:0x873ca
    at <anonymous>:wasm-function[522]:0x7d4ce
    at Function.fromRandom (/-----/node_modules/.pnpm/bsv-wasm@1.2.1/node_modules/bsv-wasm/bsv_wasm.js:2133:24)
    at Object.<anonymous> (/-----/test.js:3:30)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:12)
    at node:internal/main/run_main_module:17:47
Firaenix commented 2 years ago

Good pick up. I'll look into this. Maybe wasm-bindgen doesnt auto free the memory when a key goes out of scope.

I'll do some tests with calling key.free()

Firaenix commented 2 years ago

I can confirm that the below code doesnt exhibit any memory leak. Maybe we can look into making some further changes to wasm-bindgen to facilitate WASM GC when JS objects go out of scope.


const { PrivateKey } = require('../../pkg/node/bsv_wasm');

let now = Date.now()
const target = 15_000_000;
for (let index = 0; index < target; index++) {
    const key = PrivateKey.fromRandom();
    key.free(); // Release the WASM memory
}

console.log(`Took ${Date.now() - now}ms for ${target} iterations`);```
Firaenix commented 2 years ago

This is actually planned to be supported by wasm-bindgen in future when Weak References are added to most browsers. https://rustwasm.github.io/wasm-bindgen/reference/weak-references.html

breavyn commented 2 years ago

So does this mean that I could build the lib for node.js today using the --weak-refs flag?

https://node.green/#ES2021-features-WeakReferences

Firaenix commented 2 years ago

Yep, I’ve been looking into a way to pass the flag to the wasm-bindgen-cli so we can bypass wasm-pack. I just haven’t had enough time to completely research it yet. 100% possible though