Fix memory leak in pipe loop reference implementation

[MattiasBuelens]

The reference implementation of ReadableStream.pipeTo leaks memory when piping a lot of chunks.

Steps to reproduce

1. Go to the project root
2. Run npm i heapdump
3. Create a file called leak.js containing the code snippet below
4. Run node --expose_gc leak.js
5. Open the two created snapshots (before.heapsnapshot and after.heapsnapshot) in Chrome DevTools
   1. Click the Memory tab
   2. Right-click in the Profiles sidebar
   3. Select Load
   4. Open the heap snapshot file
6. Select the after profile and switch to the Comparison view

leak.js

'use strict';

const { ReadableStream } = require('./reference-implementation/lib/readable-stream.js');
const { WritableStream } = require('./reference-implementation/lib/writable-stream.js');
const heapdump = require('heapdump');

const delay = ms => new Promise(resolve => setTimeout(resolve, ms));

(async () => {
  let controller;
  const readable = new ReadableStream({
    start(c) {
      controller = c;
    }
  });
  const writable = new WritableStream(); // no-op sink
  const promise = readable.pipeTo(writable);

  gc();
  heapdump.writeSnapshot('./before.heapsnapshot');

  for (let i = 0; i < 1e4; i++) {
    controller.enqueue(i);
  }
  await delay(0);

  gc();
  heapdump.writeSnapshot('./after.heapsnapshot');

  controller.close();
  await promise;
})();

Expected results

• The difference in total memory usage is small.
• The difference in amount of Promise instances (# Delta) is zero.

Actual results

• The difference in total memory usage is ~1 MB.
• The difference in amount of Promise instances is 10,000.

Root cause analysis

The cause of this memory leak is due to pipeLoop creating a long promise chain using recursion:

function pipeLoop() {
  if (shuttingDown === true) {
    return Promise.resolve();
  }

  return writer._readyPromise.then(() => {
      // snipped
  })
  .then(pipeLoop); // <<< recursion!
}

Every time a chunk is piped through, the promise chain is extended. This chain keeps a ton of promises alive, and cannot be garbage collected until the pipe ends (i.e. when either end closes or errors).

Proposed solution

This PR fixes the memory leak by creating a single Promise for the return value of pipeLoop, and manually resolving/rejecting it.

With this fix, the leak is gone:

Environment

• Windows 10 1803
• Node 10.13.0

[MattiasBuelens]

I know the reference implementation isn't meant to be very optimized, but I still think it shouldn't leak memory like this. (And I don't really want to maintain a fork just to fix this leak for my stream polyfill. 😛 )

Chrome's implementation is not affected. Its internal pump does not use the promises returned by thenPromise to create a chain. I don't think any other browser has already implemented pipeTo, so they cannot be affected yet.

[ricea]

Excellent analysis. Fix looks good to me. I'm going to wait and see if @domenic has any comment.

[MattiasBuelens]

Side note: this will definitely have conflicts with #966. You can merge #966 first, I'll rebase this PR afterwards.