Skip to content

stream: reset awaitDrain after manual .resume()

Rodrigo Muino Tomonari requested to merge github/fork/addaleax/fix-7159 into master
Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

stream

Description of change

Reset the readableState.awaitDrain counter after manual calls to .resume().

What might happen otherwise is that a slow consumer at the end of the pipe could end up stalling the piping in the following scenario:

  1. The writable stream indicates that its buffer is full.
  2. This leads the readable stream to pause() and increase its awaitDrain counter, which will be decreased by the writable’s next drain event.
  3. Something calls .resume() manually.
  4. The readable continues to pipe to the writable, but once again the writable stream indicates that the buffer is full.
  5. The awaitDrain counter is thus increased again, but since it has now been increased twice for a single piping destination, the next drain event will not be able to reset awaitDrain to zero.
  6. The pipe is stalled and no data is passed along anymore.

The solution in this commit is to reset the awaitDrain counter to zero when resume() is called.

Fixes: https://github.com/nodejs/node/issues/7159

This bug is has been around at least since 0.12, and I don’t currently know why it never popped up before #2325 when the awaitDrain mechanism worked like it does today.

/cc @nodejs/streams

Merge request reports