-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
v8::Isolate::AttachCppHeap
is deprecated
#52718
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
Comments
|
Ping. This is soon going to be a problem. |
It seems to me that we can just skip the attach and detach call at all and V8 can handle it for us? (We previously already do "if there isn't one, create one, otherwise use the existing one" which seems to be what V8 does internally now) cc @mlippautz |
Or specifically is this branch always true now? Then we can just remove the else branch and the branch in |
It's entirely possible that this is easy to fix. I just am not able to do it myself with my knowledge of C++. |
I can check later to see if that branch is always true now. If it is we can delete the extra handling. |
I pushed a buildable version of the |
Moved to core repo as the code is now in |
Related: v8#209 |
It seems V8 fork is able to just let the isolate own the CppHeap now? Not sure if the task runner timing issue has been dealt with already in the upstream, if it is we can revive #53205 (comment) |
Another one: v8#210 |
And v8#211 |
And v8#212 |
And v8#213 |
I see that on the main branch, AttachCppHeap and DetachCppHeap are still only marked as V8_DEPRECATED_SOON, which means the code that are needed to fully migrate may not be there yet (see #53205 (comment)). Though when 13.4 upgrade lands we may be able to start migration. |
Closes: nodejs#52718 Co-authored-by: Andreas Haas <ahaas@chromium.org>
Closes: nodejs#52718 Co-authored-by: Andreas Haas <ahaas@chromium.org>
Closes: nodejs#52718 Co-authored-by: Andreas Haas <ahaas@chromium.org>
Closes: nodejs#52718 Co-authored-by: Andreas Haas <ahaas@chromium.org>
See: https://github.com/nodejs/node-v8/actions/runs/6742272834/job/18328109270
Refs: v8/v8@d5631f6
This breaks us here: https://github.com/nodejs/node-v8/blob/1ccbbfef3a32dbce7d5f35f17c2280bc995af10e/src/env.cc#L552
And here: https://github.com/nodejs/node-v8/blob/1ccbbfef3a32dbce7d5f35f17c2280bc995af10e/src/env.cc#L583
/cc @nodejs/cpp-reviewers @dharesign
The text was updated successfully, but these errors were encountered: