No adapter implements napi_add_env_cleanup_hook, forcing per-environment native state to be process-global #215
Description
Summary
There is no portable way to run native cleanup when a JS environment is torn down, because no Node-API adapter in Core/Node-API/Source implements napi_add_env_cleanup_hook:
$ grep -rn "napi_add_env_cleanup_hook" Core/Node-API/Source
(no matches)
The only reference anywhere in the repo is the vendored node-addon-api header Core/Node-API/Include/Shared/napi/napi-inl.h, which calls it from Napi::Env::AddCleanupHook. That call has no implementation behind it in js_native_api_chakra.cc, js_native_api_javascriptcore.cc, js_native_api_quickjs.cc or js_native_api_v8.cc.
Impact
Native polyfill state that logically belongs to an environment has to be made process-global instead, and then leaks for the life of the process.
The concrete case is the blob URL store added in #207 (Polyfills/URL/Source/URL.cpp). URL.createObjectURL registers an entry that is normally released by URL.revokeObjectURL. Entries that are never revoked should be released when the environment goes away -- that is what a browser does at unload. With no teardown hook the store must be process-global, so an embedder that creates and destroys environments over the process lifetime accumulates every un-revoked blob URL, and the buffers behind them, until the process exits.
Polyfills/URL/Include/Babylon/Polyfills/URL.h documents the store as process-global for exactly this reason, and RegisterObjectURL/RevokeObjectURL already take a Napi::Env that is currently unused, kept for future per-env scoping.
Suggested fix
Implement napi_add_env_cleanup_hook and napi_remove_env_cleanup_hook in the adapters, then scope the blob URL store per environment and drop its entries from the cleanup hook.
Notes
Not covered by the existing issues: #194 was the JSC napi_typeof bug, #213 was the UrlLib blob: scheme resolver (implemented in #207), and #214 is the UrlLib Apple lifetime bug.
Activity
Share the Blob's buffer with the blob URL store, and correct the stor…
Keep the blob URL store's C++ surface internal.
bghgary commentedon Jul 28, 2026
[Commented by Copilot on behalf of @bghgary]
Two updates now that #207 has settled its API surface.
The Impact section is out of date.
RegisterObjectURL/RevokeObjectURLno longer take aNapi::Env, and they are no longer exported at all: they are file-local functions in the anonymous namespace ofPolyfills/URL/Source/URL.cpp, since nothing outside that file calls them.Polyfills/URL/Include/Babylon/Polyfills/URL.his back to declaring onlyInitialize, so it no longer documents the store; that rationale now lives in the comment above the registry inURL.cpp.Worth adding to the suggested fix: scoping the store per environment should also remove its
std::mutex, and the separategeneratorMutexguarding thestd::mt19937_64used to mint URLs. Both exist only because the store is process-global and therefore reachable from several environments' JS threads at once (eachAppRuntimeruns its own thread). Once the store is per-environment, every access is on that environment's JS thread and the locking is dead weight.Add blob: URL support to URL.createObjectURL / revokeObjectURL (#207)