No adapter implements napi_add_env_cleanup_hook, forcing per-environment native state to be process-global

[bkaradzic-microsoft]

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.

[bghgary]

[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/RevokeObjectURL no longer take a Napi::Env, and they are no longer exported at all: they are file-local functions in the anonymous namespace of Polyfills/URL/Source/URL.cpp, since nothing outside that file calls them. Polyfills/URL/Include/Babylon/Polyfills/URL.h is back to declaring only Initialize, so it no longer documents the store; that rationale now lives in the comment above the registry in URL.cpp.

Worth adding to the suggested fix: scoping the store per environment should also remove its std::mutex, and the separate generatorMutex guarding the std::mt19937_64 used to mint URLs. Both exist only because the store is process-global and therefore reachable from several environments' JS threads at once (each AppRuntime runs its own thread). Once the store is per-environment, every access is on that environment's JS thread and the locking is dead weight.