Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small memory leak in @pm2/io due to incorrect event used #5216

Open
owenallenaz opened this issue Nov 1, 2021 · 0 comments
Open

Small memory leak in @pm2/io due to incorrect event used #5216

owenallenaz opened this issue Nov 1, 2021 · 0 comments

Comments

@owenallenaz
Copy link

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. In this case, you'll likely leak the startTime, although there is the possibility of leaking the whole closure depending on whether v8 optimizes it away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant