Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QuickJSContext had no callback with id error when newFunction was called many times #98

Closed
rot1024 opened this issue Feb 18, 2023 · 4 comments

Comments

@rot1024
Copy link

rot1024 commented Feb 18, 2023

const ctx = (await getQuickJS()).newContext();

for (let i = 0; i < 50000; i++) {
  const fn = ctx.newFunction("", () => {});
  ctx.unwrapResult(ctx.callFunction(fn, ctx.undefined));
  fn.dispose();
}

ctx.dispose();

prints

[C to host error: returning null] Error: QuickJSContext had no callback with id -15536
    at Object.callFunction (/quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/context.js:115:27)
    at /quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/module.js:36:31
    at QuickJSModuleCallbacks.handleAsyncify (/quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/module.js:145:23)
    at QuickJSEmscriptenModuleCallbacks.callFunction (/quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/module.js:30:80)
    at o (/quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/generated/emscripten-module.WASM_RELEASE_SYNC.js:281:62)
    at wasm://wasm/00174c1a:wasm-function[1105]:0x48b6d
    at wasm://wasm/00174c1a:wasm-function[1077]:0x4823d
    at wasm://wasm/00174c1a:wasm-function[238]:0xbb70
    at wasm://wasm/00174c1a:wasm-function[41]:0x1f06
    at wasm://wasm/00174c1a:wasm-function[822]:0x3aa8a
    at a._QTS_Call [as QTS_Call] (/quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/generated/emscripten-module.WASM_RELEASE_SYNC.js:335:68)
    at /quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/context.js:468:49
    at Lifetime.consume (/quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/lifetime.js:61:24)
    at QuickJSContext.callFunction (/quickjs-emscripten-sync/node_modules/quickjs-emscripten/dist/context.js:468:14)
    at /quickjs-emscripten-sync/src/edge.test.ts:63:26
    at async runTest (/quickjs-emscripten-sync/node_modules/vitest/dist/chunk-runtime-error.1104e45a.mjs:488:5)
    at async runSuite (/quickjs-emscripten-sync/node_modules/vitest/dist/chunk-runtime-error.1104e45a.mjs:576:13)
    at async runFiles (/quickjs-emscripten-sync/node_modules/vitest/dist/chunk-runtime-error.1104e45a.mjs:620:5)
    at async startTestsNode (/quickjs-emscripten-sync/node_modules/vitest/dist/chunk-runtime-error.1104e45a.mjs:638:3)
    at async /quickjs-emscripten-sync/node_modules/vitest/dist/entry.mjs:76:9
    at async Module.withEnv (/quickjs-emscripten-sync/node_modules/vitest/dist/chunk-runtime-error.1104e45a.mjs:259:5)
    at async run (/quickjs-emscripten-sync/node_modules/vitest/dist/entry.mjs:71:5)
    at async file:///quickjs-emscripten-sync/node_modules/tinypool/dist/esm/worker.js:99:20

(this test code was executed on vitest)

Note that If the number of repetitions is small, no error occurs.

Although 50,000 times may seem like a lot, it is an error that is realistically encountered in a code of a certain size.

  • MacOS Venture 13.2 on MacBook Pro M1 2020
  • quickjs-emscripten: v0.21.0 and v0.21.1
  • Node.js: v18.11.0 and Google Chrome 110.0.5481.100(Official Build) (arm64)
@justjake
Copy link
Owner

The limitation is due to QuickJS internally having only a uint_16 for "magic" on each function we create. I recently merged an improvement to this, but I didn't release it: #94

@justjake
Copy link
Owner

With #94 released, you'll have a limit of 65535 functions. Beyond that you'll need to do indirection in user-space. Meaning, if you've created 65534 functions (limit - 1), you should create a function that takes a function ID as a JS Number value, and maps that to a host function by reading the number.

@rot1024
Copy link
Author

rot1024 commented Feb 19, 2023

Thank you for reply.

I also got that idea and implemented it in quickjs-emscripten-sync.

reearth/quickjs-emscripten-sync#21

It calls newFunction only once and it saves all funcImpl to the map with a number. I will try it and confirm if it works in our environment.

@justjake
Copy link
Owner

Sounds good 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants