diff --git a/src/env.h b/src/env.h index 96b0cf52fb2..d52c0d92a32 100644 --- a/src/env.h +++ b/src/env.h @@ -322,11 +322,12 @@ class EnabledDebugList; class StreamReadSlab { public: // Sized to match the read buffer size that libuv suggests for stream - // reads: a slab typically serves a single large read (still avoiding the - // copy that right-sizing the buffer would need), or many small ones. - // Larger slabs amortize allocations further, but stay alive (pinned by - // chunk views) long enough to be promoted to V8's old generation, where - // their external memory is only reclaimed by major GCs. + // reads: a slab serves a single large read (still avoiding the copy that + // right-sizing the buffer would need), or is reused for small reads that + // are copied out of it. Larger slabs amortize allocations further, but + // stay alive (pinned by chunk views) long enough to be promoted to V8's + // old generation, where their external memory is only reclaimed by major + // GCs. static constexpr size_t kSlabSize = 64 * 1024; // Reserve `suggested` bytes. Starts a new slab if the current one does diff --git a/src/stream_base.cc b/src/stream_base.cc index a73a39513fb..f36a4b7056b 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -725,6 +725,12 @@ void StreamResource::ClearError() { uv_buf_t EmitToJSStreamListener::OnStreamAlloc(size_t suggested_size) { CHECK_NOT_NULL(stream_); Environment* env = static_cast(stream_)->stream_env(); + // Reads that ask for less than a slab (e.g. decrypted TLS records, which + // are allocated at their exact size) get their own buffer: as views over + // a shared slab, any one of them kept by JS would keep the whole slab + // alive, along with every other read that was carved out of it. + if (suggested_size < StreamReadSlab::kSlabSize) + return env->allocate_managed_buffer(suggested_size); return env->stream_read_slab().Allocate(env->isolate(), suggested_size); } diff --git a/test/parallel/test-tls-small-read-own-buffer.js b/test/parallel/test-tls-small-read-own-buffer.js new file mode 100644 index 00000000000..cc65e92f736 --- /dev/null +++ b/test/parallel/test-tls-small-read-own-buffer.js @@ -0,0 +1,46 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Small TLS reads must not be views over a larger shared buffer: JS keeping +// one of them would keep the whole buffer, and every other read in it, alive. + +const assert = require('assert'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const kBlock = 320; +const kBlocks = 3; + +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}, common.mustCall((socket) => { + let received = 0; + socket.on('data', common.mustCallAtLeast((chunk) => { + assert.strictEqual(chunk.byteOffset, 0); + assert.strictEqual(chunk.buffer.byteLength, chunk.length); + received += chunk.length; + if (received === kBlocks * kBlock) { + socket.end(); + server.close(); + } else if (received % kBlock === 0) { + // Ask for the next block only once this one has arrived, so that each + // block is a separate small read. + socket.write('n'); + } + })); +})); + +server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + rejectUnauthorized: false, + }, common.mustCall(() => { + client.write(Buffer.alloc(kBlock)); + })); + client.on('data', common.mustCall(() => { + client.write(Buffer.alloc(kBlock)); + }, kBlocks - 1)); +}));