diff --git a/benchmark/repl/completion.js b/benchmark/repl/completion.js new file mode 100644 index 000000000000..b9f55d8416b0 --- /dev/null +++ b/benchmark/repl/completion.js @@ -0,0 +1,53 @@ +'use strict'; + +const common = require('../common.js'); +const repl = require('node:repl'); +const { PassThrough, Writable } = require('node:stream'); + +const bench = common.createBenchmark(main, { + n: [5e3], + query: [ + 'cons', + 'console.lo', + 'Buffer.prototype.wri', + "require('f", + ], + useGlobal: [0, 1], +}); + +function main({ n, query, useGlobal }) { + const input = new PassThrough(); + const output = new Writable({ write(c, e, cb) { cb(); } }); + const server = new repl.REPLServer({ + input, + output, + terminal: false, + useGlobal: !!useGlobal, + }); + + // Inspector callbacks do not keep the event loop alive on their own. + const keepAlive = setInterval(() => {}, 0x7fffffff); + let remaining = n; + + function complete() { + server.complete(query, onComplete); + } + + function onComplete(err) { + if (err) { + throw err; + } + + if (--remaining === 0) { + bench.end(n); + clearInterval(keepAlive); + server.close(); + return; + } + + setImmediate(complete); + } + + bench.start(); + setImmediate(complete); +} diff --git a/benchmark/repl/creation.js b/benchmark/repl/creation.js new file mode 100644 index 000000000000..60e795063d19 --- /dev/null +++ b/benchmark/repl/creation.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common.js'); +const repl = require('node:repl'); +const { PassThrough, Writable } = require('node:stream'); + +const bench = common.createBenchmark(main, { + n: [500], + preview: [0, 1], + terminal: [0, 1], + useGlobal: [0, 1], +}, { + combinationFilter: ({ preview, terminal }) => !!terminal || !preview, +}); + +function main({ n, preview, terminal, useGlobal }) { + const inputs = Array.from({ length: n }, () => new PassThrough()); + const outputs = Array.from( + { length: n }, + () => new Writable({ write(c, e, cb) { cb(); } }), + ); + const servers = new Array(n); + + bench.start(); + for (let i = 0; i < n; i++) { + servers[i] = new repl.REPLServer({ + input: inputs[i], + output: outputs[i], + preview: !!preview, + terminal: !!terminal, + useGlobal: !!useGlobal, + }); + } + bench.end(n); + + for (const server of servers) { + server.close(); + } +} diff --git a/benchmark/repl/evaluate.js b/benchmark/repl/evaluate.js new file mode 100644 index 000000000000..48376a2b48fe --- /dev/null +++ b/benchmark/repl/evaluate.js @@ -0,0 +1,57 @@ +'use strict'; + +const common = require('../common.js'); +const repl = require('node:repl'); +const { PassThrough, Writable } = require('node:stream'); + +const bench = common.createBenchmark(main, { + n: [2e4], + code: [ + '1 + 1', + '({ answer: 42 })', + 'Promise.resolve(42)', + 'await Promise.resolve(42)', + ], + mode: ['sloppy', 'strict'], + useGlobal: [0, 1], +}); + +function main({ n, code, mode, useGlobal }) { + const input = new PassThrough(); + const output = new Writable({ write(c, e, cb) { cb(); } }); + const server = new repl.REPLServer({ + input, + output, + replMode: mode === 'strict' ? + repl.REPL_MODE_STRICT : + repl.REPL_MODE_SLOPPY, + terminal: false, + useGlobal: !!useGlobal, + }); + + // Inspector callbacks do not keep the event loop alive on their own. + const keepAlive = setInterval(() => {}, 0x7fffffff); + let remaining = n; + + function evaluate() { + server.eval(`${code}\n`, server.context, 'repl', onEvaluate); + } + + function onEvaluate(err) { + if (err) { + throw err; + } + + if (--remaining === 0) { + bench.end(n); + clearInterval(keepAlive); + server.close(); + return; + } + + setImmediate(evaluate); + } + + bench.start(); + setImmediate(evaluate); +} diff --git a/benchmark/repl/process-lines.js b/benchmark/repl/process-lines.js new file mode 100644 index 000000000000..fd019512f9fc --- /dev/null +++ b/benchmark/repl/process-lines.js @@ -0,0 +1,52 @@ +'use strict'; + +const common = require('../common.js'); +const repl = require('node:repl'); +const { PassThrough, Writable } = require('node:stream'); + +const bench = common.createBenchmark(main, { + n: [1e4], + code: [ + '1 + 1\n', + 'Promise.resolve(42)\n', + ], + mode: ['sloppy', 'strict'], + terminal: [0, 1], + useGlobal: [0, 1], +}); + +function main({ n, code: inputCode, mode, terminal, useGlobal }) { + const input = new PassThrough(); + const output = new Writable({ write(c, e, cb) { cb(); } }); + const server = new repl.REPLServer({ + input, + output, + replMode: mode === 'strict' ? + repl.REPL_MODE_STRICT : + repl.REPL_MODE_SLOPPY, + terminal: !!terminal, + useGlobal: !!useGlobal, + }); + const originalEval = server.eval; + // TTY input dispatch can briefly have no other active event loop handles. + const keepAlive = setInterval(() => {}, 0x7fffffff); + let remaining = n; + + // eslint-disable-next-line node-core/func-name-matching + server.eval = function REPLEval(code, context, file, callback) { + originalEval(code, context, file, function onEvaluate() { + const result = Reflect.apply(callback, this, arguments); + if (--remaining === 0) { + bench.end(n); + clearInterval(keepAlive); + server.close(); + } else { + setImmediate(() => input.write(inputCode)); + } + return result; + }); + }; + + bench.start(); + input.write(inputCode); +} diff --git a/benchmark/repl/reset-context.js b/benchmark/repl/reset-context.js new file mode 100644 index 000000000000..ab96f92564f2 --- /dev/null +++ b/benchmark/repl/reset-context.js @@ -0,0 +1,26 @@ +'use strict'; + +const common = require('../common.js'); +const repl = require('node:repl'); +const { PassThrough, Writable } = require('node:stream'); + +const bench = common.createBenchmark(main, { + n: [1e3], +}); + +function main({ n }) { + const input = new PassThrough(); + const output = new Writable({ write(c, e, cb) { cb(); } }); + const server = new repl.REPLServer({ + input, + output, + terminal: false, + }); + + bench.start(); + for (let i = 0; i < n; i++) { + server.resetContext(); + } + bench.end(n); + server.close(); +} diff --git a/lib/_http_agent.js b/lib/_http_agent.js index f4da2ed246cd..3871c5b56a2f 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -280,10 +280,17 @@ function maybeEnableKeylog(eventName) { this[kOnKeylog] = function onkeylog(keylog) { agent.emit('keylog', keylog, this); }; - // Existing sockets will start listening on keylog now. - const sockets = ObjectValues(this.sockets); - for (let i = 0; i < sockets.length; i++) { - sockets[i].on('keylog', this[kOnKeylog]); + // Existing sockets will start listening on keylog now. Both maps hold + // arrays of sockets keyed by name, so each bucket has to be walked. + const sets = [this.freeSockets, this.sockets]; + for (let s = 0; s < sets.length; s++) { + const buckets = ObjectValues(sets[s]); + for (let b = 0; b < buckets.length; b++) { + const sockets = buckets[b]; + for (let n = 0; n < sockets.length; n++) { + sockets[n].on('keylog', this[kOnKeylog]); + } + } } } } diff --git a/lib/internal/ffi-shared-buffer.js b/lib/internal/ffi-shared-buffer.js index c8b48d2aad4c..94764bdf0db7 100644 --- a/lib/internal/ffi-shared-buffer.js +++ b/lib/internal/ffi-shared-buffer.js @@ -198,6 +198,8 @@ function inheritMetadata(wrapper, rawFn, nargs) { // arguments out of it into invocation-local storage before `ffi_call` and // reads the return value back only after, so nested/reentrant calls into // the same function are safe. +// Concise methods do not have [[Construct]], unlike function expressions, so +// use them below to match the native FFI functions' non-constructible behavior. function wrapWithSharedBuffer(rawFn, signature) { if (rawFn == null) return rawFn; const buffer = rawFn[kSbSharedBuffer]; @@ -254,7 +256,7 @@ function wrapWithSharedBuffer(rawFn, signature) { // so arity specialization wouldn't buy much here. assert(slowInvoke !== undefined, 'FFI: shared-buffer raw function with pointer arguments is missing kSbInvokeSlow'); - wrapper = function(...args) { + wrapper = { invoke(...args) { if (args.length !== nargs) { throwFFIArgCountError(nargs, args.length); } @@ -271,7 +273,7 @@ function wrapWithSharedBuffer(rawFn, signature) { } rawFn(); return retGetter === null ? undefined : retGetter(view, 0, true); - }; + } }.invoke; } else { // Arity specialization avoids the per-call `Array` allocation of // `...args`; the void/non-void split removes a per-call branch on @@ -295,42 +297,42 @@ function buildNumericWrapper( /* c8 ignore start */ if (nargs === 0) { if (retGetter === null) { - return function() { + return { invoke() { if (arguments.length !== 0) { throwFFIArgCountError(0, arguments.length); } rawFn(); - }; + } }.invoke; } - return function() { + return { invoke() { if (arguments.length !== 0) { throwFFIArgCountError(0, arguments.length); } rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } /* c8 ignore stop */ if (nargs === 1) { const i0 = argInfos[0]; const o0 = argOffsets[0]; if (retGetter === null) { - return function(a0) { + return { invoke(a0) { if (arguments.length !== 1) { throwFFIArgCountError(1, arguments.length); } writeNumericArg(view, i0, o0, a0, 0); rawFn(); - }; + } }.invoke; } - return function(a0) { + return { invoke(a0) { if (arguments.length !== 1) { throwFFIArgCountError(1, arguments.length); } writeNumericArg(view, i0, o0, a0, 0); rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } if (nargs === 2) { const i0 = argInfos[0]; @@ -338,16 +340,16 @@ function buildNumericWrapper( const o0 = argOffsets[0]; const o1 = argOffsets[1]; if (retGetter === null) { - return function(a0, a1) { + return { invoke(a0, a1) { if (arguments.length !== 2) { throwFFIArgCountError(2, arguments.length); } writeNumericArg(view, i0, o0, a0, 0); writeNumericArg(view, i1, o1, a1, 1); rawFn(); - }; + } }.invoke; } - return function(a0, a1) { + return { invoke(a0, a1) { if (arguments.length !== 2) { throwFFIArgCountError(2, arguments.length); } @@ -355,7 +357,7 @@ function buildNumericWrapper( writeNumericArg(view, i1, o1, a1, 1); rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } if (nargs === 3) { const i0 = argInfos[0]; @@ -365,7 +367,7 @@ function buildNumericWrapper( const o1 = argOffsets[1]; const o2 = argOffsets[2]; if (retGetter === null) { - return function(a0, a1, a2) { + return { invoke(a0, a1, a2) { if (arguments.length !== 3) { throwFFIArgCountError(3, arguments.length); } @@ -373,9 +375,9 @@ function buildNumericWrapper( writeNumericArg(view, i1, o1, a1, 1); writeNumericArg(view, i2, o2, a2, 2); rawFn(); - }; + } }.invoke; } - return function(a0, a1, a2) { + return { invoke(a0, a1, a2) { if (arguments.length !== 3) { throwFFIArgCountError(3, arguments.length); } @@ -384,7 +386,7 @@ function buildNumericWrapper( writeNumericArg(view, i2, o2, a2, 2); rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } if (nargs === 4) { const i0 = argInfos[0]; @@ -396,7 +398,7 @@ function buildNumericWrapper( const o2 = argOffsets[2]; const o3 = argOffsets[3]; if (retGetter === null) { - return function(a0, a1, a2, a3) { + return { invoke(a0, a1, a2, a3) { if (arguments.length !== 4) { throwFFIArgCountError(4, arguments.length); } @@ -405,9 +407,9 @@ function buildNumericWrapper( writeNumericArg(view, i2, o2, a2, 2); writeNumericArg(view, i3, o3, a3, 3); rawFn(); - }; + } }.invoke; } - return function(a0, a1, a2, a3) { + return { invoke(a0, a1, a2, a3) { if (arguments.length !== 4) { throwFFIArgCountError(4, arguments.length); } @@ -417,7 +419,7 @@ function buildNumericWrapper( writeNumericArg(view, i3, o3, a3, 3); rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } if (nargs === 5) { const i0 = argInfos[0]; @@ -431,7 +433,7 @@ function buildNumericWrapper( const o3 = argOffsets[3]; const o4 = argOffsets[4]; if (retGetter === null) { - return function(a0, a1, a2, a3, a4) { + return { invoke(a0, a1, a2, a3, a4) { if (arguments.length !== 5) { throwFFIArgCountError(5, arguments.length); } @@ -441,9 +443,9 @@ function buildNumericWrapper( writeNumericArg(view, i3, o3, a3, 3); writeNumericArg(view, i4, o4, a4, 4); rawFn(); - }; + } }.invoke; } - return function(a0, a1, a2, a3, a4) { + return { invoke(a0, a1, a2, a3, a4) { if (arguments.length !== 5) { throwFFIArgCountError(5, arguments.length); } @@ -454,7 +456,7 @@ function buildNumericWrapper( writeNumericArg(view, i4, o4, a4, 4); rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } if (nargs === 6) { const i0 = argInfos[0]; @@ -470,7 +472,7 @@ function buildNumericWrapper( const o4 = argOffsets[4]; const o5 = argOffsets[5]; if (retGetter === null) { - return function(a0, a1, a2, a3, a4, a5) { + return { invoke(a0, a1, a2, a3, a4, a5) { if (arguments.length !== 6) { throwFFIArgCountError(6, arguments.length); } @@ -481,9 +483,9 @@ function buildNumericWrapper( writeNumericArg(view, i4, o4, a4, 4); writeNumericArg(view, i5, o5, a5, 5); rawFn(); - }; + } }.invoke; } - return function(a0, a1, a2, a3, a4, a5) { + return { invoke(a0, a1, a2, a3, a4, a5) { if (arguments.length !== 6) { throwFFIArgCountError(6, arguments.length); } @@ -495,12 +497,12 @@ function buildNumericWrapper( writeNumericArg(view, i5, o5, a5, 5); rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } // 7+ args: further specialization is diminishing returns and bloats // this builder. if (retGetter === null) { - return function(...args) { + return { invoke(...args) { if (args.length !== nargs) { throwFFIArgCountError(nargs, args.length); } @@ -508,9 +510,9 @@ function buildNumericWrapper( writeNumericArg(view, argInfos[i], argOffsets[i], args[i], i); } rawFn(); - }; + } }.invoke; } - return function(...args) { + return { invoke(...args) { if (args.length !== nargs) { throwFFIArgCountError(nargs, args.length); } @@ -519,7 +521,7 @@ function buildNumericWrapper( } rawFn(); return retGetter(view, 0, true); - }; + } }.invoke; } module.exports = { diff --git a/lib/internal/ffi/fast-api.js b/lib/internal/ffi/fast-api.js index b93d061b00cd..4355ab1a992f 100644 --- a/lib/internal/ffi/fast-api.js +++ b/lib/internal/ffi/fast-api.js @@ -254,6 +254,8 @@ function throwIfFastLibraryClosed(state) { } } +// Concise methods do not have [[Construct]], unlike function expressions. +// Keep wrappers non-constructible to match the native FFI functions. function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { if (rawFn === undefined || rawFn === null) { return rawFn; @@ -287,7 +289,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { const memory0 = needsRawPointerConversion(t0) || string0; const fastBufferInvoke = needsPointerLikeConversion(t0) ? rawFn[kFastBufferInvoke] : undefined; - wrapper = function(a0) { + wrapper = { invoke(a0) { throwIfFastLibraryClosed(state); if (arguments.length !== 1) { throwFFIArgCountError(1, arguments.length); @@ -312,13 +314,13 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { arg = getRawPointerArg(arg, 0); } return rawFn(arg); - }; + } }.invoke; } else if (nargs === 2) { const c0 = ArrayPrototypeIncludes(indexes, 0); const c1 = ArrayPrototypeIncludes(indexes, 1); const t0 = argumentTypes[0]; const t1 = argumentTypes[1]; - wrapper = function(a0, a1) { + wrapper = { invoke(a0, a1) { throwIfFastLibraryClosed(state); if (arguments.length !== 2) { throwFFIArgCountError(2, arguments.length); @@ -332,7 +334,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { } finally { if (stringCall) exitStringConversion(stringState); } - }; + } }.invoke; } else if (nargs === 3) { const c0 = ArrayPrototypeIncludes(indexes, 0); const c1 = ArrayPrototypeIncludes(indexes, 1); @@ -340,7 +342,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { const t0 = argumentTypes[0]; const t1 = argumentTypes[1]; const t2 = argumentTypes[2]; - wrapper = function(a0, a1, a2) { + wrapper = { invoke(a0, a1, a2) { throwIfFastLibraryClosed(state); if (arguments.length !== 3) { throwFFIArgCountError(3, arguments.length); @@ -356,9 +358,9 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { } finally { if (stringCall) exitStringConversion(stringState); } - }; + } }.invoke; } else { - wrapper = function(...args) { + wrapper = { invoke(...args) { throwIfFastLibraryClosed(state); if (args.length !== nargs) { throwFFIArgCountError(nargs, args.length); @@ -382,7 +384,7 @@ function wrapWithRawPointerConversions(rawFn, argumentTypes, owner) { } finally { if (stringCall) exitStringConversion(stringState); } - }; + } }.invoke; } return inheritMetadata(wrapper, rawFn, nargs); diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 4c53f7ea23e1..882aa8e71fc9 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -158,7 +158,19 @@ function importFd(stream, options) { stream[kHandle] = options.fd; stream[kFs] = FileHandleOperations(stream[kHandle]); stream[kHandle][kRef](); - options.fd.on('close', FunctionPrototypeBind(stream.close, stream)); + + const onclose = FunctionPrototypeBind(stream.close, stream); + options.fd.on('close', onclose); + if (options.autoClose === false) { + function cleanup() { + options.fd.removeListener('close', onclose); + options.fd[kUnref](); + } + stream.once('end', cleanup); + stream.once('finish', cleanup); + stream.once('error', cleanup); + } + return options.fd.fd; } diff --git a/src/api/callback.cc b/src/api/callback.cc index 0f458f470501..85da82ff84a5 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -245,7 +245,7 @@ MaybeLocal InternalMakeCallback(Environment* env, Local context = env->context(); if (use_async_hooks_trampoline) { - MaybeStackBuffer, 16> args(3 + argc); + MaybeStackBuffer args(env->isolate(), 3 + argc); args[0] = Number::New(env->isolate(), asyncContext.async_id); args[1] = resource; args[2] = callback; diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index bf47c239abbe..9aa77e5c8574 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -187,12 +187,12 @@ Local AddrTTLToArray( Environment* env, const T* addrttls, size_t naddrttls) { - MaybeStackBuffer, 8> ttls(naddrttls); + MaybeStackBuffer ttls(env->isolate(), naddrttls); for (size_t i = 0; i < naddrttls; i++) { ttls[i] = Integer::NewFromUnsigned(env->isolate(), addrttls[i].ttl); } - return Array::New(env->isolate(), ttls.out(), naddrttls); + return ttls.ToArray(); } // Parse the CSV produced by ares_get_servers_csv() back into (ip, port) diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 936c201de99d..5a531209e232 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -41,7 +41,6 @@ using ncrypto::MarkPopErrorOnReturn; using ncrypto::SSLPointer; using ncrypto::SSLSessionPointer; using ncrypto::X509Pointer; -using v8::Array; using v8::ArrayBuffer; using v8::ArrayBufferView; using v8::BackingStore; @@ -1956,7 +1955,7 @@ void TLSWrap::GetSharedSigalgs(const FunctionCallbackInfo& args) { SSL* ssl = w->ssl_.get(); int nsig = SSL_get_shared_sigalgs(ssl, 0, nullptr, nullptr, nullptr, nullptr, nullptr); - MaybeStackBuffer, 16> ret_arr(nsig); + MaybeStackBuffer ret_arr(env->isolate(), nsig); for (int i = 0; i < nsig; i++) { int hash_nid; @@ -2023,8 +2022,7 @@ void TLSWrap::GetSharedSigalgs(const FunctionCallbackInfo& args) { ret_arr[i] = OneByteString(env->isolate(), sig_with_md); } - args.GetReturnValue().Set( - Array::New(env->isolate(), ret_arr.out(), ret_arr.length())); + args.GetReturnValue().Set(ret_arr.ToArray()); } void TLSWrap::ExportKeyingMaterial(const FunctionCallbackInfo& args) { diff --git a/src/js_stream.cc b/src/js_stream.cc index 0d6d6a7ac80a..3a3a1c943d25 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -11,7 +11,6 @@ namespace node { using errors::TryCatchScope; -using v8::Array; using v8::Context; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -119,17 +118,14 @@ int JSStream::DoWrite(WriteWrap* w, int value_int = UV_EPROTO; - MaybeStackBuffer, 16> bufs_arr(count); + MaybeStackBuffer bufs_arr(env()->isolate(), count); for (size_t i = 0; i < count; i++) { if (!Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocal(&bufs_arr[i])) { return value_int; } } - Local argv[] = { - w->object(), - Array::New(env()->isolate(), bufs_arr.out(), count) - }; + Local argv[] = {w->object(), bufs_arr.ToArray()}; TryCatchScope try_catch(env()); Local value; diff --git a/src/js_udp_wrap.cc b/src/js_udp_wrap.cc index 1648d162b8f7..83b91042023f 100644 --- a/src/js_udp_wrap.cc +++ b/src/js_udp_wrap.cc @@ -11,7 +11,6 @@ namespace node { using errors::TryCatchScope; -using v8::Array; using v8::Context; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -98,7 +97,7 @@ ssize_t JSUDPWrap::Send(uv_buf_t* bufs, int64_t value_int = JS_EXCEPTION_PENDING; size_t total_len = 0; - MaybeStackBuffer, 16> buffers(nbufs); + MaybeStackBuffer buffers(env()->isolate(), nbufs); for (size_t i = 0; i < nbufs; i++) { if (!Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocal(&buffers[i])) { return value_int; @@ -110,9 +109,9 @@ ssize_t JSUDPWrap::Send(uv_buf_t* bufs, if (!AddressToJS(env(), addr).ToLocal(&address)) return value_int; Local args[] = { - listener()->CreateSendWrap(total_len)->object(), - Array::New(env()->isolate(), buffers.out(), nbufs), - address, + listener()->CreateSendWrap(total_len)->object(), + buffers.ToArray(), + address, }; if (!MakeCallback(env()->onwrite_string(), arraysize(args), args) diff --git a/src/node_concepts.h b/src/node_concepts.h index 5dbce5c6f88a..3109e2ab83f8 100644 --- a/src/node_concepts.h +++ b/src/node_concepts.h @@ -3,6 +3,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include #include #include #include @@ -32,6 +33,12 @@ concept StandardCharType = template concept IsCallable = std::is_function::value || requires { &T::operator(); }; +// Types that can reside on V8's managed heap (v8::Value, v8::Object, etc.). +// Used to select the MaybeStackBuffer specialization that holds handles in a +// v8::LocalVector instead of malloc'd memory. +template +concept V8Type = std::is_base_of_v; + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/node_dir.cc b/src/node_dir.cc index 952161d9e2cf..eca5140b6423 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -206,7 +206,7 @@ static MaybeLocal DirentListToArray(Environment* env, uv_dirent_t* ents, int num, enum encoding encoding) { - MaybeStackBuffer, 64> entries(num * 2); + MaybeStackBuffer entries(env->isolate(), num * 2); // Return an array of all read filenames. int j = 0; @@ -222,7 +222,8 @@ static MaybeLocal DirentListToArray(Environment* env, entries[j++] = Integer::New(env->isolate(), ents[i].type); } - return Array::New(env->isolate(), entries.out(), j); + CHECK_EQ(j, num * 2); + return entries.ToArray(); } static void AfterDirRead(uv_fs_t* req) { diff --git a/src/node_env_var.cc b/src/node_env_var.cc index e94180cd659d..25e405be86d2 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -201,7 +201,7 @@ MaybeLocal RealEnvStore::Enumerate(Isolate* isolate) const { auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); }); CHECK_EQ(uv_os_environ(&items, &count), 0); - MaybeStackBuffer, 256> env_v(count); + MaybeStackBuffer env_v(isolate, count); int env_v_index = 0; for (int i = 0; i < count; i++) { #ifdef _WIN32 @@ -216,7 +216,10 @@ MaybeLocal RealEnvStore::Enumerate(Isolate* isolate) const { env_v[env_v_index++] = str; } - return Array::New(isolate, env_v.out(), env_v_index); + // We're possibly not filling the entire buffer. + CHECK_LE(env_v_index, count); + env_v.SetLength(env_v_index); + return env_v.ToArray(); } std::shared_ptr KVStore::Clone(Isolate* isolate) const { diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 42c62c829168..7e3da95b958f 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -32,7 +32,6 @@ using v8::Local; using v8::LocalVector; using v8::Maybe; using v8::MaybeLocal; -using v8::Null; using v8::Object; using v8::PropertyAttribute; using v8::ReadOnly; @@ -324,7 +323,9 @@ MaybeLocal DynamicLibrary::CreateFunction( maybe_ret = Function::New(context, use_sb ? DynamicLibrary::InvokeFunctionSB : DynamicLibrary::InvokeFunction, - info->object()); + info->object(), + 0, + v8::ConstructorBehavior::kThrow); } Local ret; @@ -377,8 +378,11 @@ MaybeLocal DynamicLibrary::CreateFunction( // (strings, Buffers, ArrayBuffers, and ArrayBufferViews). if (has_ptr_args) { Local slow_fn; - if (!Function::New( - context, DynamicLibrary::InvokeFunction, info->object()) + if (!Function::New(context, + DynamicLibrary::InvokeFunction, + info->object(), + 0, + v8::ConstructorBehavior::kThrow) .ToLocal(&slow_fn)) { return MaybeLocal(); } @@ -710,13 +714,11 @@ void DynamicLibrary::InvokeCallback(ffi_cif* cif, size_t expected_args = cb->args.size(); LocalVector callback_args(isolate, expected_args); + // libffi always points `args[i]` at its own storage for the value of + // argument `i`, so the slot pointers themselves are never null. A NULL + // pointer argument surfaces as the BigInt `0n` via ToJSArgument. for (size_t i = 0; i < expected_args; i++) { - if (args[i] == nullptr) { - callback_args[i] = Null(isolate); - continue; - } else { - callback_args[i] = ToJSArgument(isolate, cb->args[i], args[i]); - } + callback_args[i] = ToJSArgument(isolate, cb->args[i], args[i]); } TryCatch try_catch(isolate); diff --git a/src/node_http2.cc b/src/node_http2.cc index 04b2acca148d..58f12c5561b5 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1609,8 +1609,8 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { // this way for performance reasons (it's faster to generate and pass an // array than it is to generate and pass the object). - MaybeStackBuffer, 64> headers_v(stream->headers_count() * 2); - MaybeStackBuffer, 32> sensitive_v(stream->headers_count()); + MaybeStackBuffer headers_v(isolate, stream->headers_count() * 2); + MaybeStackBuffer sensitive_v(isolate, stream->headers_count()); size_t sensitive_count = 0; stream->TransferHeaders([&](const Http2Header& header, size_t i) { @@ -1627,13 +1627,14 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { stream->retained_headers_length_ += stream->current_headers_length_; stream->current_headers_length_ = 0; + sensitive_v.SetLength(sensitive_count); Local args[] = { - stream->object(), - Integer::New(isolate, id), - Integer::New(isolate, stream->headers_category()), - Integer::New(isolate, frame->hd.flags), - Array::New(isolate, headers_v.out(), headers_v.length()), - Array::New(isolate, sensitive_v.out(), sensitive_count), + stream->object(), + Integer::New(isolate, id), + Integer::New(isolate, stream->headers_category()), + Integer::New(isolate, frame->hd.flags), + headers_v.ToArray(), + sensitive_v.ToArray(), }; MakeCallback(env()->http2session_on_headers_function(), arraysize(args), args); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index f00ab803fef0..5a0c4962d3f7 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1093,7 +1093,7 @@ void MessagePort::PostMessage(const FunctionCallbackInfo& args) { "MessagePort.postMessage"); } - TransferList transfer_list; + TransferList transfer_list(env->isolate()); if (!GetTransferList(env, context, args[1], &transfer_list)) { return; } @@ -1607,7 +1607,7 @@ static void StructuredClone(const FunctionCallbackInfo& args) { Local value = args[0]; - TransferList transfer_list; + TransferList transfer_list(isolate); Local options = args[1].As(); Local transfer_list_v; if (!options->Get(context, env->transfer_string()) diff --git a/src/node_messaging.h b/src/node_messaging.h index 3a838a39200a..0abd7da3bf02 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -17,7 +17,7 @@ namespace worker { class MessagePortData; class MessagePort; -typedef MaybeStackBuffer, 8> TransferList; +typedef MaybeStackBuffer TransferList; // Used to represent the in-flight structure of an object that is being // transferred or cloned using postMessage(). diff --git a/src/node_v8.cc b/src/node_v8.cc index 34e8460790b7..14ee626c9f3e 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -745,17 +745,17 @@ void Initialize(Local target, // Heap space names are extracted once and exposed to JavaScript to // avoid excessive creation of heap space name Strings. HeapSpaceStatistics s; - MaybeStackBuffer, 16> heap_spaces(number_of_heap_spaces); + MaybeStackBuffer heap_spaces(env->isolate(), + number_of_heap_spaces); for (size_t i = 0; i < number_of_heap_spaces; i++) { env->isolate()->GetHeapSpaceStatistics(&s, i); heap_spaces[i] = String::NewFromUtf8(env->isolate(), s.space_name()) .ToLocalChecked(); } target - ->Set( - context, - FIXED_ONE_BYTE_STRING(env->isolate(), "kHeapSpaces"), - Array::New(env->isolate(), heap_spaces.out(), number_of_heap_spaces)) + ->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "kHeapSpaces"), + heap_spaces.ToArray()) .Check(); SetMethod(context, diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 2da2e18950ad..0358bb995fee 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -767,7 +767,7 @@ MaybeLocal SyncProcessRunner::BuildOutputArray() { CHECK(!stdio_pipes_.empty()); EscapableHandleScope scope(env()->isolate()); - MaybeStackBuffer, 8> js_output(stdio_pipes_.size()); + MaybeStackBuffer js_output(env()->isolate(), stdio_pipes_.size()); for (uint32_t i = 0; i < stdio_pipes_.size(); i++) { SyncProcessStdioPipe* h = stdio_pipes_[i].get(); @@ -781,8 +781,7 @@ MaybeLocal SyncProcessRunner::BuildOutputArray() { } } - return scope.Escape( - Array::New(env()->isolate(), js_output.out(), js_output.length())); + return scope.Escape(js_output.ToArray()); } Maybe SyncProcessRunner::ParseOptions(Local js_value) { diff --git a/src/util-inl.h b/src/util-inl.h index 0a5a90def488..7eb5ebabbfea 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -394,14 +394,13 @@ v8::MaybeLocal ToV8Value(v8::Local context, if (isolate == nullptr) isolate = v8::Isolate::GetCurrent(); v8::EscapableHandleScope handle_scope(isolate); - MaybeStackBuffer, 128> arr(vec.size()); - arr.SetLength(vec.size()); + MaybeStackBuffer arr(isolate, vec.size()); for (size_t i = 0; i < vec.size(); ++i) { if (!ToV8Value(context, vec[i], isolate).ToLocal(&arr[i])) return v8::MaybeLocal(); } - return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length())); + return handle_scope.Escape(arr.ToArray()); } template @@ -430,8 +429,7 @@ v8::MaybeLocal ToV8Value(v8::Local context, if (isolate == nullptr) isolate = v8::Isolate::GetCurrent(); v8::EscapableHandleScope handle_scope(isolate); - MaybeStackBuffer, 128> arr(vec.size()); - arr.SetLength(vec.size()); + MaybeStackBuffer arr(isolate, vec.size()); auto it = vec.begin(); for (size_t i = 0; i < vec.size(); ++i) { if (!ToV8Value(context, *it, isolate).ToLocal(&arr[i])) @@ -439,7 +437,7 @@ v8::MaybeLocal ToV8Value(v8::Local context, std::advance(it, 1); } - return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length())); + return handle_scope.Escape(arr.ToArray()); } template @@ -519,7 +517,14 @@ v8::Local ToV8ValuePrimitiveArray(v8::Local context, } SlicedArguments::SlicedArguments( - const v8::FunctionCallbackInfo& args, size_t start) { + const v8::FunctionCallbackInfo& args, size_t start) + : SlicedArguments(args.GetIsolate(), args, start) {} + +SlicedArguments::SlicedArguments( + v8::Isolate* isolate, + const v8::FunctionCallbackInfo& args, + size_t start) + : MaybeStackBuffer(isolate) { const size_t length = static_cast(args.Length()); if (start >= length) return; const size_t size = length - start; @@ -545,6 +550,27 @@ void MaybeStackBuffer::AllocateSufficientStorage( length_ = storage; } +template +void MaybeStackBuffer::AllocateSufficientStorage( + size_t storage) { + CHECK(!IsInvalidated()); + if (storage > capacity()) { + if (!local_vector_.has_value()) { + local_vector_.emplace(isolate_, storage); + // Copy existing stack data into the LocalVector. + for (size_t i = 0; i < length_; i++) { + (*local_vector_)[i] = buf_st_[i]; + } + } else { + local_vector_->resize(storage); + } + buf_ = local_vector_->data(); + capacity_ = storage; + } + + length_ = storage; +} + template requires(sizeof(T) == 1) ArrayBufferViewContents::ArrayBufferViewContents( diff --git a/src/util.h b/src/util.h index 0461a5ebb19f..621d2fbf4ee6 100644 --- a/src/util.h +++ b/src/util.h @@ -526,6 +526,85 @@ class MaybeStackBuffer { T buf_st_[kStackStorageSize]; }; +template +class MaybeStackBuffer { + public: + using V = v8::Local; + + MaybeStackBuffer(const MaybeStackBuffer&) = delete; + MaybeStackBuffer& operator=(const MaybeStackBuffer& other) = delete; + + const V* out() const { return buf_; } + V* out() { return buf_; } + + // operator* for compatibility with `v8::String::(Utf8)Value` + V* operator*() { return buf_; } + const V* operator*() const { return buf_; } + + V& operator[](size_t index) { + CHECK_LT(index, length()); + return buf_[index]; + } + + const V& operator[](size_t index) const { + CHECK_LT(index, length()); + return buf_[index]; + } + + size_t length() const { return length_; } + + // Current maximum capacity of the buffer with which SetLength() can be used + // without first calling AllocateSufficientStorage(). + size_t capacity() const { return capacity_; } + + // Make sure enough space for `storage` entries is available. + // This method can be called multiple times throughout the lifetime of the + // buffer, but once this has been called Invalidate() cannot be used. + // Content of the buffer in the range [0, length()) is preserved. + void AllocateSufficientStorage(size_t storage); + + void SetLength(size_t length) { + // capacity() returns how much memory is actually available. + CHECK_LE(length, capacity()); + length_ = length; + } + + // If the buffer is stored in a LocalVector rather than on the stack. + bool IsAllocated() const { return !IsInvalidated() && buf_ != buf_st_; } + + // If Invalidate() has been called. + bool IsInvalidated() const { return buf_ == nullptr; } + + explicit MaybeStackBuffer(v8::Isolate* isolate) + : isolate_(isolate), + length_(0), + capacity_(arraysize(buf_st_)), + buf_(buf_st_) { + // Default to a zero-length, null-terminated buffer. + buf_[0] = V(); + } + + MaybeStackBuffer(v8::Isolate* isolate, size_t storage) + : MaybeStackBuffer(isolate) { + AllocateSufficientStorage(storage); + } + + // LocalVector (via optional) handles cleanup automatically. + ~MaybeStackBuffer() = default; + + v8::Local ToArray() const { + return v8::Array::New(isolate_, buf_, length_); + } + + private: + v8::Isolate* isolate_; + size_t length_; + size_t capacity_; + V* buf_; + V buf_st_[kStackStorageSize]; + std::optional> local_vector_; +}; + // Provides access to an ArrayBufferView's storage, either the original, // or for small data, a copy of it. This object's lifetime is bound to the // original ArrayBufferView's lifetime. @@ -782,10 +861,13 @@ constexpr inline bool IsBigEndian() { static_assert(IsLittleEndian() || IsBigEndian(), "Node.js does not support mixed-endian systems"); -class SlicedArguments : public MaybeStackBuffer> { +class SlicedArguments : public MaybeStackBuffer { public: inline explicit SlicedArguments( const v8::FunctionCallbackInfo& args, size_t start = 0); + inline SlicedArguments(v8::Isolate* isolate, + const v8::FunctionCallbackInfo& args, + size_t start = 0); }; // Convert a v8::PersistentBase, e.g. v8::Global, to a Local, with an extra diff --git a/test/benchmark/test-benchmark-repl.js b/test/benchmark/test-benchmark-repl.js new file mode 100644 index 000000000000..045813039589 --- /dev/null +++ b/test/benchmark/test-benchmark-repl.js @@ -0,0 +1,8 @@ +'use strict'; + +const common = require('../common'); +const runBenchmark = require('../common/benchmark'); + +common.skipIfInspectorDisabled(); + +runBenchmark('repl', { NODEJS_BENCHMARK_ZERO_ALLOWED: 1 }); diff --git a/test/ffi/test-ffi-dynamic-library.js b/test/ffi/test-ffi-dynamic-library.js index 82400335a12f..d22cf48d77b1 100644 --- a/test/ffi/test-ffi-dynamic-library.js +++ b/test/ffi/test-ffi-dynamic-library.js @@ -67,6 +67,27 @@ test('dlopen resolves functions from definitions', () => { } }); +test('FFI functions are not constructible', () => { + const { lib, functions } = ffi.dlopen(libraryPath, { + add_i32: fixtureSymbols.add_i32, + multiply_f64: fixtureSymbols.multiply_f64, + }); + + try { + assert.strictEqual(Object.hasOwn(functions.add_i32, 'prototype'), false); + assert.strictEqual( + Object.hasOwn(functions.multiply_f64, 'prototype'), false); + assert.throws( + () => Reflect.construct(functions.add_i32, [20, 22]), + TypeError); + assert.throws( + () => Reflect.construct(functions.multiply_f64, [6, 7]), + TypeError); + } finally { + lib.close(); + } +}); + test('DynamicLibrary exposes functions and symbols', () => { const lib = new ffi.DynamicLibrary(libraryPath); diff --git a/test/parallel/test-fs-promises-file-handle-stream.js b/test/parallel/test-fs-promises-file-handle-stream.js index 71f312b6f9d7..61d0b3ca2ec7 100644 --- a/test/parallel/test-fs-promises-file-handle-stream.js +++ b/test/parallel/test-fs-promises-file-handle-stream.js @@ -42,7 +42,46 @@ async function validateRead() { ); } +async function validateReusedCreateReadStream() { + const filePath = path.resolve(tmpDir, 'tmp-reused-stream.txt'); + fs.writeFileSync(filePath, Buffer.from('ab', 'utf8')); + + const fileHandle = await open(filePath, 'r'); + try { + await buffer(fileHandle.createReadStream({ + start: 0, + end: 0, + autoClose: false, + })); + assert.strictEqual(fileHandle.listenerCount('close'), 0); + + await buffer(fileHandle.createReadStream({ + start: 1, + end: 1, + autoClose: false, + })); + assert.strictEqual(fileHandle.listenerCount('close'), 0); + } finally { + await fileHandle.close(); + } +} + +async function validateReusedCreateWriteStream() { + const filePath = path.resolve(tmpDir, 'tmp-reused-write-stream.txt'); + const fileHandle = await open(filePath, 'w'); + try { + const stream = fileHandle.createWriteStream({ autoClose: false }); + stream.end('a'); + await finished(stream); + assert.strictEqual(fileHandle.listenerCount('close'), 0); + } finally { + await fileHandle.close(); + } +} + Promise.all([ validateWrite(), validateRead(), + validateReusedCreateReadStream(), + validateReusedCreateWriteStream(), ]).then(common.mustCall()); diff --git a/test/parallel/test-http-agent-keylog-existing-sockets.js b/test/parallel/test-http-agent-keylog-existing-sockets.js new file mode 100644 index 000000000000..1a391c09b8be --- /dev/null +++ b/test/parallel/test-http-agent-keylog-existing-sockets.js @@ -0,0 +1,71 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +// Adding a 'keylog' listener to an agent is wired up by maybeEnableKeylog(), +// which attaches the agent's keylog handler to the sockets the agent already +// owns. `agent.sockets` and `agent.freeSockets` map a name to an *array* of +// sockets, so each bucket has to be walked. Treating the buckets themselves as +// sockets threw a TypeError out of `agent.on('keylog', ...)`, which also meant +// the listener was never registered. + +// Two servers so the two sockets get different names, which keeps one parked +// in freeSockets instead of being reused by the second request. +const idleServer = http.createServer((req, res) => res.end('idle')); +const busyServer = http.createServer((req, res) => { + setTimeout(() => res.end('busy'), common.platformTimeout(200)); +}); + +function countSockets(agent) { + let free = 0; + let active = 0; + for (const bucket of Object.values(agent.freeSockets)) free += bucket.length; + for (const bucket of Object.values(agent.sockets)) active += bucket.length; + return { free, active }; +} + +idleServer.listen(0, common.mustCall(() => { + busyServer.listen(0, common.mustCall(() => { + const agent = new http.Agent({ keepAlive: true, maxSockets: 4 }); + + // First request finishes, so its socket is released into freeSockets. + http.get({ port: idleServer.address().port, agent }, common.mustCall((res) => { + res.resume(); + res.on('end', common.mustCall(() => { + // Second request is still in flight, so its socket is in sockets. + const req = http.get({ port: busyServer.address().port, agent }, + common.mustCall((res2) => { + res2.resume(); + res2.on('end', common.mustCall(() => { + agent.destroy(); + idleServer.close(); + busyServer.close(); + })); + })); + + req.on('socket', common.mustCall(() => { + setImmediate(common.mustCall(() => { + const { free, active } = countSockets(agent); + assert.strictEqual(free, 1); + assert.strictEqual(active, 1); + + // Used to throw `TypeError: sockets[i].on is not a function`. + agent.on('keylog', common.mustNotCall()); + assert.strictEqual(agent.listenerCount('keylog'), 1); + + // Every existing socket, idle or in use, is now listening. + for (const set of [agent.freeSockets, agent.sockets]) { + for (const bucket of Object.values(set)) { + for (const socket of bucket) { + assert.strictEqual(socket.listenerCount('keylog'), 1); + } + } + } + })); + })); + })); + })); + })); +}));