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

v8::Global handles in ContextState are preventing realms from being GC'd #17199

Open
andreubotella opened this issue Dec 27, 2022 · 0 comments
Open

Comments

@andreubotella
Copy link
Contributor

andreubotella commented Dec 27, 2022

In my recent work to reland support for realms in deno_core, I have re-added the ContextState struct, and started populating it with v8::Global<v8::Function> handles that were previously in JsRuntimeState. ContextState instances are stored in a slot of v8::Context, which means they will be dropped when the corresponding v8::Context is garbage collected, or when the isolate is dropped, whichever comes earlier.

However, the v8::Global handles in ContextState point to Deno.core.* functions inside the corresponding V8 context, and therefore have a reference to the context's Function.prototype built-in, which in turn has a reference to the context itself. Therefore such handles cause a reference cycle. But this is not a cycle that V8's garbage collector can detect, because V8 has no way to inspect the contents of the slots associated to a v8::Context, and therefore no way to detect that it is keeping any v8::Globals alive. Thus, the corresponding V8 context will never be garbage collected, and ContextState instances will only be dropped when the isolate is dropped.

This was also an issue before support for realms was reverted in #16366, but it was not noticed at the time.

In Deno CLI applications (as well as Deploy, I assume), this is not yet an issue since there is only one realm per isolate, and the JsRuntime keeps a strong reference to it in any case. But it will become a problem once we support ShadowRealm (see denoland/deno_core#911 and #16211). We should then probably use v8::Weak instead of v8::Global for those handles.

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

1 participant