Skip to content

Memory leak due to res.once("finish") #283

@owenallenaz

Description

@owenallenaz

At https://github.com/keymetrics/pm2-io-apm/blob/master/src/metrics/httpMetrics.ts#L178 you have the following passage:

return function (event: string, req: any, res: any) {
  // only handle http request
  if (event !== 'request') return original.apply(this, arguments)

  const meter: Meter | undefined = self.metrics.get(`${name}.meter`)
  if (meter !== undefined) {
    meter.mark()
  }
  const latency: Histogram | undefined = self.metrics.get(`${name}.latency`)
  if (latency === undefined) return original.apply(this, arguments)
  if (res === undefined || res === null) return original.apply(this, arguments)
  const startTime = Date.now()
  // wait for the response to set the metrics
  res.once('finish', _ => {
    latency.update(Date.now() - startTime)
  })
  return original.apply(this, arguments)
}

res.once is only called once the response has been flushed to the connecting socket. If the socket was to hang-up, such as a timeout on the client side, it results in the finish event never occurring. Instead, you likely want to use the close event. You can see a replication of this in the following gist: https://gist.github.com/owenallenaz/bc66547842e87a649b554b7c168a7314 . If you run that server and hit it with call, and then hit the /status/ route you'll see that finish is not called for every cancelled request. This has the potentiality to leak the res object in it's entirety.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions