BabylonJS / BabylonJS/JsRuntimeHost

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

Open
#215 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
22
Forks
23
Avg merge
3d 14h
Merged PRs (30d)
6

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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.