From a05f72f9ca48986841e503e59fd7d3d10d3db343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Thu, 20 Jul 2023 15:20:44 +0200 Subject: [PATCH 1/2] test: delete test-net-bytes-per-incoming-chunk-overhead The test's assumptions about RSS are no longer valid, at least with Fedora 38. Closes: https://github.com/nodejs/node/issues/48490 PR-URL: https://github.com/nodejs/node/pull/48811 Fixes: https://github.com/nodejs/node/issues/48490 Reviewed-By: Luigi Pinca Reviewed-By: Richard Lau --- ...t-net-bytes-per-incoming-chunk-overhead.js | 50 ------------------- 1 file changed, 50 deletions(-) delete mode 100644 test/pummel/test-net-bytes-per-incoming-chunk-overhead.js diff --git a/test/pummel/test-net-bytes-per-incoming-chunk-overhead.js b/test/pummel/test-net-bytes-per-incoming-chunk-overhead.js deleted file mode 100644 index b3613110ab5c54..00000000000000 --- a/test/pummel/test-net-bytes-per-incoming-chunk-overhead.js +++ /dev/null @@ -1,50 +0,0 @@ -// Flags: --expose-gc -'use strict'; - -const common = require('../common'); - -if (process.config.variables.asan) { - common.skip('ASAN messes with memory measurements'); -} - -if (common.isPi) { - common.skip('Too slow for Raspberry Pi devices'); -} - -const assert = require('assert'); -const net = require('net'); - -// Tests that, when receiving small chunks, we do not keep the full length -// of the original allocation for the libuv read call in memory. - -let client; -let baseRSS; -const receivedChunks = []; -const N = 250000; - -const server = net.createServer(common.mustCall((socket) => { - baseRSS = process.memoryUsage.rss(); - - socket.setNoDelay(true); - socket.on('data', (chunk) => { - receivedChunks.push(chunk); - if (receivedChunks.length < N) { - client.write('a'); - } else { - client.end(); - server.close(); - } - }); -})).listen(0, common.mustCall(() => { - client = net.connect(server.address().port); - client.setNoDelay(true); - client.write('hello!'); -})); - -process.on('exit', () => { - global.gc(); - const bytesPerChunk = - (process.memoryUsage.rss() - baseRSS) / receivedChunks.length; - // We should always have less than one page (usually ~ 4 kB) per chunk. - assert(bytesPerChunk < 650, `measured ${bytesPerChunk} bytes per chunk`); -}); From eb29d9a6c505405c4b19d18198d480bc79a3fdcd Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 24 Jul 2023 09:47:47 +0200 Subject: [PATCH 2/2] url: ensure getter access do not mutate observable symbols PR-URL: https://github.com/nodejs/node/pull/48897 Refs: https://github.com/nodejs/node/pull/48891 Refs: https://github.com/nodejs/node/issues/48886 Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow --- lib/internal/url.js | 30 +++++++++++++------ .../test-whatwg-url-custom-searchparams.js | 2 ++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 51688b8403076a..beadc12cbaa913 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -22,6 +22,7 @@ const { ReflectOwnKeys, RegExpPrototypeSymbolReplace, SafeMap, + SafeWeakMap, StringPrototypeCharAt, StringPrototypeCharCodeAt, StringPrototypeCodePointAt, @@ -99,6 +100,12 @@ const FORWARD_SLASH = /\//g; const context = Symbol('context'); const searchParams = Symbol('query'); +/** + * We cannot use private fields without a breaking change, so a WeakMap is the alternative. + * @type {WeakMap} + */ +const internalSearchParams = new SafeWeakMap(); + const updateActions = { kProtocol: 0, kHost: 1, @@ -179,7 +186,7 @@ class URLContext { } function isURLSearchParams(self) { - return self && self[searchParams] && !self[searchParams][searchParams]; + return self?.[searchParams]; } class URLSearchParams { @@ -672,11 +679,12 @@ class URL { ctx.hash_start = hash_start; ctx.scheme_type = scheme_type; - if (this[searchParams]) { + const alreadyInstantiatedSearchParams = internalSearchParams.get(this); + if (alreadyInstantiatedSearchParams) { if (ctx.hasSearch) { - this[searchParams][searchParams] = parseParams(this.search); + alreadyInstantiatedSearchParams[searchParams] = parseParams(this.search); } else { - this[searchParams][searchParams] = []; + alreadyInstantiatedSearchParams[searchParams] = []; } } } @@ -898,11 +906,15 @@ class URL { get searchParams() { if (!isURL(this)) throw new ERR_INVALID_THIS('URL'); - if (this[searchParams] == null) { - this[searchParams] = new URLSearchParams(this.search); - this[searchParams][context] = this; - } - return this[searchParams]; + + const cachedValue = internalSearchParams.get(this); + if (cachedValue != null) + return cachedValue; + + const value = new URLSearchParams(this.search); + value[context] = this; + internalSearchParams.set(this, value); + return value; } get hash() { diff --git a/test/parallel/test-whatwg-url-custom-searchparams.js b/test/parallel/test-whatwg-url-custom-searchparams.js index 0b2087ac246313..75fa1779bdeb45 100644 --- a/test/parallel/test-whatwg-url-custom-searchparams.js +++ b/test/parallel/test-whatwg-url-custom-searchparams.js @@ -16,7 +16,9 @@ const normalizedValues = ['a', '1', 'true', 'undefined', 'null', '\uFFFD', '[object Object]']; const m = new URL('http://example.org'); +const ownSymbolsBeforeGetterAccess = Object.getOwnPropertySymbols(m); const sp = m.searchParams; +assert.deepStrictEqual(Object.getOwnPropertySymbols(m), ownSymbolsBeforeGetterAccess); assert(sp); assert.strictEqual(sp.toString(), '');