summaryrefslogtreecommitdiff
path: root/src/node_contextify.cc
Commit message (Collapse)AuthorAgeFilesLines
* vm: fix crash when setting __proto__ on context's globalThisFeng Yu2023-05-151-1/+2
| | | | | | | PR-URL: https://github.com/nodejs/node/pull/47939 Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
* bootstrap: initialize per-isolate properties of bindings separatelyJoyee Cheung2023-05-031-42/+43
| | | | | | | | | | | | | | This patch moves the initialization of per-isolate properties of the bindings that are in the embedded snapshot separate from the initialization of their per-context properties. This is necessary for workers to share the isolate snapshot with the main thread and deserialize these properties instead of creating them from scratch. PR-URL: https://github.com/nodejs/node/pull/47768 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
* vm: properly handle defining props on any valueNicolas DUBIEN2023-03-181-3/+15
| | | | | | | | | | | While it was supposed to fix most of the remaining issues, https://github.com/nodejs/node/pull/46458 missed some in strict mode. This PR adds some additional checks. It also clarifies what we are really checking to execute or not the `GetReturnValue`. PR-URL: https://github.com/nodejs/node/pull/46615 Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
* Revert "vm: fix leak in vm.compileFunction when importModuleDynamically is used"Michaël Zasso2023-03-151-5/+12
| | | | | | | | | | | | | | | | This reverts commit 986498b7b329f454507b3a47e9f20cbcdb029dc2. Fixes: https://github.com/nodejs/node/issues/47096 PR-URL: https://github.com/nodejs/node/pull/47101 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Danielle Adams <adamzdanielle@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
* vm: fix leak in vm.compileFunction when importModuleDynamically is usedJoyee Cheung2023-03-011-12/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously in the implementation there was a cycle that V8 could not detect: Strong global reference to CompiledFnEntry (JS wrapper) -> strong reference to callback setting (through the callbackMap key-value pair) -> importModuleDynamically (wrapper in internalCompileFunction()) -> Strong reference to the compiled function (through closure in internalCompileFunction()) The CompiledFnEntry only gets GC'ed when the compiled function is GC'ed. Since the compiled function is always reachable as described above, there is a leak. We only needed the first strong global reference because we didn't want the function to outlive the CompiledFnEntry. In this case it can be solved by using a private symbol instead of going with the global reference + destruction in the weak callback, which V8's GC is not going to understand. PR-URL: https://github.com/nodejs/node/pull/46785 Fixes: https://github.com/nodejs/node/issues/42080 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
* lib,src: fix a few typos in commentsTobias Nießen2023-02-251-1/+1
| | | | | | | | PR-URL: https://github.com/nodejs/node/pull/46835 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
* vm: properly support symbols on globalsNicolas DUBIEN2023-02-041-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | A regression has been introduced in node 18.2.0, it makes the following snippet fails while it used to work in the past: ``` const assert = require('assert'); const vm = require('vm'); const global = vm.runInContext('this', vm.createContext()); const totoSymbol = Symbol.for('toto'); Object.defineProperty(global, totoSymbol, { enumerable: true, writable: true, value: 4, configurable: true, }); assert(Object.getOwnPropertySymbols(global).includes(totoSymbol)); ``` Regression introduced by: https://github.com/nodejs/node/pull/42963. So I basically attempted to start understanding what it changed to make it fix the initial issue while not breaking the symbol related one. Fixes: https://github.com/nodejs/node/issues/45983 PR-URL: https://github.com/nodejs/node/pull/46458 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
* src: add snapshot support for embedder APIAnna Henningsen2023-02-031-3/+1
| | | | | | | | | | | | | | | | | | | Add experimental support for loading snapshots in the embedder API by adding a public opaque wrapper for our `SnapshotData` struct and allowing embedders to pass it to the relevant setup functions. Where applicable, use these helpers to deduplicate existing code in Node.js’s startup path. This has shown a 40 % startup performance increase for a real-world application, even with the somewhat limited current support for built-in modules. The documentation includes a note about no guarantees for API or ABI stability for this feature while it is experimental. PR-URL: https://github.com/nodejs/node/pull/45888 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
* vm: expose cachedDataRejected for vm.compileFunctionAnna Henningsen2023-01-251-49/+76
| | | | | | | | | | | | Having this information available is useful for functions just as it is for scripts. Therefore, expose it in the same way that other information related to code caching is reported. As part of this, de-duplify the code for setting the properties on the C++ side and add proper exception handling to it. PR-URL: https://github.com/nodejs/node/pull/46320 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
* src: rename internal module declaration as internal bindingslegendecas2022-11-291-3/+3
| | | | | | | | | | | | | | | | This is a continuation of the name reification on the internal bindings. Renames NODE_MODULE_CONTEXT_AWARE_INTERNAL and NODE_MODULE_EXTERNAL_REFERENCE to NODE_BINDING_CONTEXT_AWARE_INTERNAL and NODE_BINDING_EXTERNAL_REFERENCE respectively. PR-URL: https://github.com/nodejs/node/pull/45551 Refs: https://github.com/nodejs/node/issues/44036 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
* src: track contexts in the Environment instead of AsyncHooksJoyee Cheung2022-11-071-2/+1
| | | | | | | | | | | | | This makes it easier to support the vm contexts in the startup snapshot. We now manage the promise hooks using references to the contexts from the Environment, and AsyncHooks only hold references to the hooks. PR-URL: https://github.com/nodejs/node/pull/45282 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
* src,lib: retrieve parsed source map url from v8legendecas2022-10-231-8/+20
| | | | | | | | | | | | | V8 already parses the source map magic comments. Currently, only scripts and functions expose the parsed source map URLs. It is unnecessary to parse the source map magic comments again when the parsed information is available. PR-URL: https://github.com/nodejs/node/pull/44798 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Jan Krems <jan.krems@gmail.com>
* vm: make ContextifyContext a BaseObjectJoyee Cheung2022-10-041-84/+113
| | | | | | | | | | | | Instead of adding a reference to the ContextifyContext by using a v8::External, we make ContextifyContext a weak BaseObject that whose wrapper is referenced by the sandbox via a private symbol. This makes it easier to snapshot the contexts, in addition to reusing the BaseObject lifetime management for ContextifyContexts. PR-URL: https://github.com/nodejs/node/pull/44796 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* src: introduce node::RealmChengzhong Wu2022-08-311-1/+1
| | | | | | | | | | To distinguish per-context values from the node::Environment, split those values to a new node::Realm structure and consolidate bootstrapping methods with it. PR-URL: https://github.com/nodejs/node/pull/44179 Refs: https://github.com/nodejs/node/issues/42528 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* vm: avoid unnecessary property getter interceptor callsJoyee Cheung2022-08-241-7/+9
| | | | | | | | | | | | | | | | | | Access to the global object from within a vm context is intercepted so it's slow, therefore we should try to avoid unnecessary access to it during the initialization of vm contexts. - Remove the Atomics.wake deletion as V8 now does not install it anymore. - Move the Intl.v8BreakIterator deletion into the snapshot. - Do not query the Object prototype if --disable-proto is not set. This should speed up the creation of vm contexts by about ~12%. PR-URL: https://github.com/nodejs/node/pull/44252 Refs: https://github.com/nodejs/node/issues/44014 Refs: https://github.com/nodejs/node/issues/37476 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
* vm: include vm context in the embedded snapshotJoyee Cheung2022-08-241-9/+24
| | | | | | | | | | Include a minimally initialized contextify context in the embedded snapshot. This paves the way for user-land vm context snapshots. PR-URL: https://github.com/nodejs/node/pull/44252 Refs: https://github.com/nodejs/node/issues/44014 Refs: https://github.com/nodejs/node/issues/37476 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
* vm: make ContextifyContext template context-independentJoyee Cheung2022-08-241-102/+124
| | | | | | | | | | | | | | | | Instead of creating an object template for every ContextifyContext, we now create one object template that can be reused by all contexts. The native pointer can be obtained through an embdder pointer field in the creation context of the receiver in the interceptors, because the interceptors are only meant to be invoked on the global object of the contextified contexts. This makes the ContextifyContext template context-independent and therefore snapshotable. PR-URL: https://github.com/nodejs/node/pull/44252 Refs: https://github.com/nodejs/node/issues/44014 Refs: https://github.com/nodejs/node/issues/37476 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
* src: use imported namespaces in `node_contextify.cc`Juan José2022-08-211-2/+2
| | | | | PR-URL: https://github.com/nodejs/node/pull/44299 Reviewed-By: Feng Yu <F3n67u@outlook.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
* src: remove usage on ScriptCompiler::CompileFunctionInContextChengzhong Wu2022-08-161-13/+14
| | | | | | | | | | | | | | V8 APIs like HostImportModuleDynamicallyCallback and ScriptCompiler::CompileFunction is moving away from ScriptOrModule. Replaces ScriptCompiler::CompileFunctionInContext with ScriptCompiler::CompileFunction to remove the usages on the optional out param ScriptOrModule. PR-URL: https://github.com/nodejs/node/pull/44198 Fixes: https://github.com/nodejs/node-v8/issues/214 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
* src: remove usages of GetBackingStore in modulesKeyhan Vakil2022-08-031-4/+2
| | | | | | | | | | | | | | This removes all usages of GetBackingStore in modules. See the linked issue for an explanation. Refs: https://github.com/nodejs/node/issues/32226 Refs: https://github.com/nodejs/node/pull/43921 PR-URL: https://github.com/nodejs/node/pull/44076 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Feng Yu <F3n67u@outlook.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
* src: split property helpers from node::Environmentlegendecas2022-08-021-15/+21
| | | | | | | | | PR-URL: https://github.com/nodejs/node/pull/44056 Refs: https://github.com/nodejs/node/issues/42528 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Feng Yu <F3n67u@outlook.com>
* lib,src: add source map support for global evallegendecas2022-07-111-1/+5
| | | | | | | | | | Dynamic sources with FunctionConstructor is not supported yet as V8 prepends lines to the sources which makes the stack line number incorrect. PR-URL: https://github.com/nodejs/node/pull/43428 Refs: https://github.com/nodejs/node/issues/43047 Reviewed-By: Ben Coe <bencoe@gmail.com>
* src: merge RunInThisContext() with RunInContext()Daeyeon Jeong2022-07-111-48/+20
| | | | | | | | | | | | | | | This commit resolves a TODO in `RunInThisContext()` by merging `RunInThisContext()` with `RunInContext()`. Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com PR-URL: https://github.com/nodejs/node/pull/43225 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
* src: replace TraceEventScope with sync eventslegendecas2022-05-161-23/+9
| | | | | | | | | | | According to the chrome trace event format document, works that are performed on one single thread should be traced with sync duration events. In this way, these events can be grouped under one thread and the trace event viewer can estimate the CPU usage of that thread. PR-URL: https://github.com/nodejs/node/pull/42977 Reviewed-By: Darshan Sen <raisinten@gmail.com>
* src: always signal V8 for intercepted propertiesMichaël Zasso2022-05-061-9/+1
| | | | | | | | Closes: https://github.com/nodejs/node/issues/42962 PR-URL: https://github.com/nodejs/node/pull/42963 Fixes: https://github.com/nodejs/node/issues/42962 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
* src: return Maybe<bool> from InitializeContextRuntime()Darshan Sen2021-08-121-1/+3
| | | | | | | | Signed-off-by: Darshan Sen <darshan.sen@postman.com> PR-URL: https://github.com/nodejs/node/pull/39695 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
* src: set PromiseHooks by EnvironmentBryan English2021-06-021-0/+5
| | | | | | | | | | | | | | | | | | | | | The new JS PromiseHooks introduced in the referenced PR are per v8::Context. This meant that code depending on them, such as AsyncLocalStorage, wouldn't behave correctly across vm.Context instances. PromiseHooks are now synchronized across the main Context and any Context created via vm.Context. Refs: https://github.com/nodejs/node/pull/36394 Fixes: https://github.com/nodejs/node/issues/38781 Signed-off-by: Bryan English <bryan@bryanenglish.com> PR-URL: https://github.com/nodejs/node/pull/38821 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
* src: replace `auto`s in node_contextify.ccXadillaX2021-05-191-3/+4
| | | | | | | | PR-URL: https://github.com/nodejs/node/pull/38644 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
* bootstrap: include vm and contextify binding into the snapshotJoyee Cheung2021-05-201-4/+35
| | | | | | | | | | | | | | In addition, defer the patching of the vm module based on the value of --experimental-vm-modules to runtime. PR-URL: https://github.com/nodejs/node/pull/38677 Refs: https://github.com/nodejs/node/issues/35711 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
* src: cache some context in localsXadillaX2021-04-191-13/+19
| | | | | | | | | Refs: https://github.com/nodejs/node/commit/66566df5773c8ef2f0e1181e8f9e47403e68c4b7 PR-URL: https://github.com/nodejs/node/pull/37473 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* src: use non-deprecated V8 module APIsMichaël Zasso2021-03-151-2/+4
| | | | | | | PR-URL: https://github.com/nodejs/node/pull/37587 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
* src: use non-deprecated V8 module and script APIsMichaël Zasso2021-02-251-20/+16
| | | | | | | PR-URL: https://github.com/nodejs/node/pull/37330 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
* src: reduce duplicated boilerplate with new env utility fnJames M Snell2021-01-031-11/+1
| | | | | | | | | Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: https://github.com/nodejs/node/pull/36536 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
* src: use correct outer Context’s microtask queueAnna Henningsen2020-12-131-1/+3
| | | | | | | | | | | | | Fall back to using the outer context’s microtask queue, rather than the Isolate’s default one. This would otherwise result in surprising behavior if an embedder specified a custom microtask queue for the main Node.js context. PR-URL: https://github.com/nodejs/node/pull/36482 Refs: https://github.com/v8/v8/commit/4bf051d536a172e7932845d82f918ad7280c7873 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
* vm: add `SafeForTerminationScope`s for SIGINT interruptionsAnna Henningsen2020-12-061-0/+1
| | | | | | | | | | | | | | | | | | | Some embedders, like Electron, choose to start Node.js with `only_terminate_in_safe_scope` set to `true`. In those cases, parts of the API that expect execution termination to happen need to be marked as able to receive those events. In our case, this is the Ctrl+C support of the `vm` module (and Workers, but since we’re in control of creating the `Isolate` for them, that’s a non-concern there). Add those scopes and add a regression test. PR-URL: https://github.com/nodejs/node/pull/36344 Reviewed-By: Shelley Vohr <codebytere@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Gus Caplan <me@gus.host>
* src: move node_contextify to modern THROW_ERR_*James M Snell2020-10-101-1/+2
| | | | | | | Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: https://github.com/nodejs/node/pull/35470 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
* vm: add run-after-evaluate microtask modeAnna Henningsen2020-06-261-7/+72
| | | | | | | | | | | | This allows timeouts to apply to e.g. `Promise`s and `async function`s from code running inside of `vm.Context`s, by giving the Context its own microtasks queue. Fixes: https://github.com/nodejs/node/issues/3020 PR-URL: https://github.com/nodejs/node/pull/34023 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com>
* src: remove unnecessary ToLocalChecked callDaniel Bevenius2020-06-221-2/+2
| | | | | | | | | PR-URL: https://github.com/nodejs/node/pull/33902 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
* vm: allow proxy callbacks to throwGus Caplan2020-06-111-6/+6
| | | | | | | | | | | Fixes: https://github.com/nodejs/node/issues/33806 PR-URL: https://github.com/nodejs/node/pull/33808 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* src: remove unused using declarationsDaniel Bevenius2020-05-281-1/+0
| | | | | | | | | | | | This commit removes the unused using declarations reported by lint-cpp. PR-URL: https://github.com/nodejs/node/pull/33268 Refs: https://github.com/nodejs/node/issues/29226 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
* vm: fix vm.measureMemory() and introduce execution optionJoyee Cheung2020-04-301-25/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | https://github.com/nodejs/node-v8/pull/147 broke the `vm.measureMemory()` API. It only created a `MeasureMemoryDelegate` and without actually calling `v8::Isolate::MeasureMemory()` so the returned promise will never resolve. This was not caught by the tests because the promise resolvers were not wrapped with `common.mustCall()`. This patch migrates the API properly and also introduce the newly added execution option to the API. It also removes support for specifying contexts to measure - instead we'll just return the measurements for all contexts in the detailed mode, which is what the `performance.measureMemory()` prototype in V8 currently does. We can consider implementing our own `v8::MeasureMemoryDelegate` to select the target context in `ShouldMeasure()` in the future, but then we'll also need to implement `MeasurementComplete()` to assemble the result. For now it's probably too early to do that. Since this API is still experimental (and guarded with a warning), such breakage should be acceptable. Refs: https://github.com/nodejs/node-v8/pull/147 PR-URL: https://github.com/nodejs/node/pull/32988 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
* src: use unique_ptr for CachedData in ContextifyScript::NewAnna Henningsen2020-04-301-2/+2
| | | | | | | | | This closes a memory leak. PR-URL: https://github.com/nodejs/node/pull/33113 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
* src: migrate measureMemory to new v8 apigengjiawen2020-03-181-6/+9
| | | | | | | PR-URL: https://github.com/nodejs/node/pull/32116 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
* src: improve handling of internal field countingJames M Snell2020-03-021-5/+9
| | | | | | | | | | | | | | | | Change suggested by bnoordhuis. Improve handing of internal field counting by using enums. Helps protect against future possible breakage if field indexes are ever changed or added to. Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: https://github.com/nodejs/node/pull/31960 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
* vm: implement vm.measureMemory() for per-context memory measurementJoyee Cheung2020-02-261-0/+44
| | | | | | | | | | | | | | | | This patch implements `vm.measureMemory()` with the new `v8::Isolate::MeasureMemory()` API to measure per-context memory usage. This should be experimental, since detailed memory measurement requires further integration with the V8 API that should be available in a future V8 update. PR-URL: https://github.com/nodejs/node/pull/31824 Refs: https://github.com/ulan/performance-measure-memory Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* vm: lazily initialize primordials for vm contextsJoyee Cheung2020-02-191-2/+5
| | | | | | | | | | | | Lazily initialize primordials when cross-context support for builtins is needed to fix the performance regression in context creation. PR-URL: https://github.com/nodejs/node/pull/31738 Fixes: https://github.com/nodejs/node/issues/29842 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>
* src: migrate off ArrayBuffer::GetContentsAnna Henningsen2019-11-121-4/+4
| | | | | | | | | | | | | | | V8 deprecates `GetContents()` in favour of `GetBackingStore()`. Update our code to reflect that. V8 also deprecates `Externalize()` and `IsExternal()`; we should be able to remove all usage of this once V8 8.0 is there. PR-URL: https://github.com/nodejs/node/pull/30339 Refs: https://github.com/v8/v8/commit/bfe3d6bce734e596e312465e207bcfd55a59fe34 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
* src: remove unused using declarationsDaniel Bevenius2019-08-231-2/+0
| | | | | | | | | | | | | | | This commit removes unused using declarations in src/node_contextify.cc. PR-URL: https://github.com/nodejs/node/pull/29222 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
* src: make `CompiledFnEntry` a `BaseObject`Anna Henningsen2019-07-221-2/+35
| | | | | | | | | | | | | | | | | | | In particular: - Move the class definition to the relevant header file, i.e. `node_contextify.h`. - Make sure that class instances are destroyed on `Environment` teardown. - Make instances of the key object traceable in heap dumps. This is particularly relevant here because our C++ script → map key mapping could introduce memory leaks when the import function metadata refers back to the script in some way. Refs: https://github.com/nodejs/node/pull/28671 PR-URL: https://github.com/nodejs/node/pull/28782 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Rich Trott <rtrott@gmail.com>
* vm: fix gc bug with modules and compiled functionsGus Caplan2019-07-191-26/+28
| | | | | | PR-URL: https://github.com/nodejs/node/pull/28671 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Guy Bedford <guybedford@gmail.com>