http: Memory Leak

[ronag]

Running this will cause the memory usage of the node process to infinitly grow...

• Version: 10.13.0
• Platform: OSX
• Subsystem:

const http = require('http')
const { spawn } = require('child_process')
const crypto = require('crypto')
const fs = require('fs')

const dst = fs.createWriteStream('./tmp')

http.createServer((req, res) => {
  req
    .on('data', buf => {
      if (!dst.write(buf)) {
        req.pause()
      }
    })
  dst.on('drain', () => {
    if (req.readable) {
      req.resume()
    }
  })
}).listen(9988, () => {
  spawn('cat', ['/dev/zero'])
    .stdout
    .pipe(http
      .request({
        hostname: 'localhost',
        port: 9988,
        path: `/file`,
        method: 'POST'
      })
    )
})

[ronag]

Backpressure seems to work, however the buf doesn't seem to be released. They seem to be allocated using some sort of external memory since they don't seem to be part of the node heap.

[ronag]

Interestingly using pipe instead of data & drain does not seem to have the same problem?

[ronag]

Seems like IncomingMessage does some strange stuff that pipe handles which is outside of the normal "stream protocol"?

[mcollina]

Does this happen on the client side or on the server side?

[ronag]

@mcollina: server side

[ronag]

Seems related to chunked encoding

[mcollina]

@ronag are you working on a PR?

[ronag]

@mcollina no, I’m trying to find the problem but so far I have no idea. This might be over my head.

[ronag]

Our findings so far seem to indicate a difference between how Readable.resume and Readable.pipe resumes/pauses streams in a way where IncomingMessage and HttpServer depends on. In particular pipeOnDrain and resume seem to differ.

e.g. if instead of calling IncomingMessage.resume() I use the following code:

req._readableState.flowing = true
while (req.read() != null) {
}

I seem to no longer have any leaks. Though I don’t dare using such a workaround in production without understanding why that helps. My suspicion is the sync vs async resume behavior which is different between resume and pipeOnDrain.

[addaleax]

@ronag Fwiw, I’ve opened a PR to address this in #26965. It would be great if you could verify that that fully works for you.

[ronag]

@mcollina I'm a little surprised this wasn't fixed/merged in/into LTS? Is that on purpose?

[mcollina]

@mcollina I'm a little surprised this wasn't fixed/merged in/into LTS? Is that on purpose?

I'm a little surprised of your question. There is no evidence of anybody blocking #26965 getting into Node 10. Likely, it seem they just need some help.