GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — buffer: fix Blob.stream() leaking source buffer · nodejs/node@ddacb3f

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

lib/internal
- blob.js
test/parallel
- test-blob-stream-gc.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎lib/internal/blob.js‎`

Copy file name to clipboardExpand all lines: lib/internal/blob.js

+17-4Lines changed: 17 additions & 4 deletions

Original file line number	Diff line number	Diff line change
`@@ -467,30 +467,40 @@ function createBlobReaderStream(reader) {`
`467`	`467`	`// There really should only be one read at a time so using an`
`468`	`468`	`// array here is purely defensive.`
`469`	`469`	`this.pendingPulls = [];`
`470`		`- // Register a wakeup callback that the C++ side can invoke`
	`470`	`+ // Lazily register a wakeup callback that the C++ side can invoke`
`471`	`471`	`// when new data is available after a STATUS_BLOCK.`
`472`		`- reader.setWakeup(() => {`
	`472`	`+ this.wakeup = () => {`
`473`	`473`	`if (this.pendingPulls.length > 0) {`
`474`	`474`	`this.readNext(c);`
`475`	`475`	`}`
`476`		`- });`
	`476`	`+ };`
`477`	`477`	`},`
`478`	`478`	`pull(c) {`
`479`	`479`	`const { promise, resolve, reject } = PromiseWithResolvers();`
	`480`	`+ if (this.pendingPulls.length === 0) {`
	`481`	`+ reader.setWakeup(this.wakeup);`
	`482`	`+ }`
`480`	`483`	`this.pendingPulls.push({ resolve, reject });`
`481`	`484`	`this.readNext(c);`
`482`	`485`	`return promise;`
`483`	`486`	`},`
	`487`	`+ clearWakeupIfIdle() {`
	`488`	`+ if (this.pendingPulls.length === 0) {`
	`489`	`+ reader.setWakeup(undefined);`
	`490`	`+ }`
	`491`	`+ },`
`484`	`492`	`readNext(c) {`
`485`	`493`	`reader.pull((status, buffer) => {`
`486`	`494`	`// If pendingPulls is empty here, the stream had to have`
`487`	`495`	`// been canceled, and we don't really care about the result.`
`488`	`496`	`// We can simply exit.`
`489`	`497`	`if (this.pendingPulls.length === 0) {`
	`498`	`+ reader.setWakeup(undefined);`
`490`	`499`	`return;`
`491`	`500`	`}`
`492`	`501`	`if (status === 0) {`
`493`	`502`	`// EOS`
	`503`	`+ reader.setWakeup(undefined);`
`494`	`504`	`c.close();`
`495`	`505`	`// This is to signal the end for byob readers`
`496`	`506`	`// see https://streams.spec.whatwg.org/#example-rbs-pull`
`@@ -502,6 +512,7 @@ function createBlobReaderStream(reader) {`
`502`	`512`	`// The read could fail for many different reasons when reading`
`503`	`513`	`// from a non-memory resident blob part (e.g. file-backed blob).`
`504`	`514`	`// The error details the system error code.`
	`515`	`+ reader.setWakeup(undefined);`
`505`	`516`	`const error =`
`506`	`517`	`lazyDOMException('The blob could not be read',`
`507`	`518`	`'NotReadableError');`
`@@ -511,7 +522,7 @@ function createBlobReaderStream(reader) {`
`511`	`522`	`return;`
`512`	`523`	`} else if (status === 2) {`
`513`	`524`	`// STATUS_BLOCK: No data available yet. The wakeup callback`
`514`		`- // registered in start() will re-invoke readNext when data`
	`525`	`+ // registered in pull() will re-invoke readNext when data`
`515`	`526`	`// arrives.`
`516`	`527`	`return;`
`517`	`528`	`}`
`@@ -531,6 +542,7 @@ function createBlobReaderStream(reader) {`
`531`	`542`	`if (this.pendingPulls.length !== 0) {`
`532`	`543`	`const pending = this.pendingPulls.shift();`
`533`	`544`	`pending.resolve();`
	`545`	`+ this.clearWakeupIfIdle();`
`534`	`546`	`}`
`535`	`547`	`return;`
`536`	`548`	`}`
`@@ -539,6 +551,7 @@ function createBlobReaderStream(reader) {`
`539`	`551`	`});`
`540`	`552`	`},`
`541`	`553`	`cancel(reason) {`
	`554`	`+ reader.setWakeup(undefined);`
`542`	`555`	`// Reject any currently pending pulls here.`
`543`	`556`	`for (const pending of this.pendingPulls) {`
`544`	`557`	`pending.reject(reason);`

Collapse file

`‎test/parallel/test-blob-stream-gc.js‎`

Copy file name to clipboard

+56Lines changed: 56 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,56 @@`
	`1`	`+// Flags: --expose-gc --no-concurrent-array-buffer-sweeping`
	`2`	`+'use strict';`
	`3`	`+`
	`4`	`+const common = require('../common');`
	`5`	`+const assert = require('assert');`
	`6`	`+const { setImmediate: setImmediatePromise } = require('timers/promises');`
	`7`	`+`
	`8`	`+const MiB = 1024 * 1024;`
	`9`	`+const iterations = 64;`
	`10`	`+const maxRetained = 16 * MiB;`
	`11`	`+`
	`12`	`+async function collectArrayBuffers() {`
	`13`	`+ for (let i = 0; i < 3; i++) {`
	`14`	`+ global.gc();`
	`15`	`+ await setImmediatePromise();`
	`16`	`+ }`
	`17`	`+}`
	`18`	`+`
	`19`	`+async function assertNoBlobStreamRetention(name, fn) {`
	`20`	`+ const buffer = Buffer.alloc(MiB);`
	`21`	`+`
	`22`	`+ await collectArrayBuffers();`
	`23`	`+ const before = process.memoryUsage().arrayBuffers;`
	`24`	`+`
	`25`	`+ for (let i = 0; i < iterations; i++) {`
	`26`	`+ await fn(buffer);`
	`27`	`+ }`
	`28`	`+`
	`29`	`+ await collectArrayBuffers();`
	`30`	`+ const retained = process.memoryUsage().arrayBuffers - before;`
	`31`	`+`
	`32`	`+ assert(`
	`33`	`+ retained < maxRetained,`
	`34`	+ `${name} retained ${retained} bytes in arrayBuffers`,
	`35`	`+ );`
	`36`	`+}`
	`37`	`+`
	`38`	`+(async () => {`
	`39`	`+ await assertNoBlobStreamRetention('unused Blob streams',`
	`40`	`+ common.mustCall(async (buffer) => {`
	`41`	`+ new Blob([buffer]).stream();`
	`42`	`+ }, iterations));`
	`43`	`+`
	`44`	`+ await assertNoBlobStreamRetention('cancelled Blob streams',`
	`45`	`+ common.mustCall(async (buffer) => {`
	`46`	`+ await new Blob([buffer]).stream()`
	`47`	`+ .cancel();`
	`48`	`+ }, iterations));`
	`49`	`+`
	`50`	`+ await assertNoBlobStreamRetention('drained Blob streams',`
	`51`	`+ common.mustCall(async (buffer) => {`
	`52`	`+ await new Response(`
	`53`	`+ new Blob([buffer]).stream(),`
	`54`	`+ ).arrayBuffer();`
	`55`	`+ }, iterations));`
	`56`	`+})().then(common.mustCall());`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.