fix memory leak caused by a bug in node with http keep-alive (#581)

src/scope/async_hooks.js

@@ -4,6 +4,10 @@ const asyncHooks = require('./async_hooks/')
 const eid = asyncHooks.executionAsyncId
 const Base = require('./base')
 const platform = require('../platform')
+const semver = require('semver')
+
+// https://github.com/nodejs/node/issues/19859
+const hasKeepAliveBug = !semver.satisfies(process.version, '^8.13 || >=10.14.2')
 
 let singleton = null
 
@@ -17,6 +21,7 @@ class Scope extends Base {
 
     this._spans = Object.create(null)
     this._types = Object.create(null)
+    this._weaks = new WeakMap()
     this._hook = asyncHooks.createHook({
       init: this._init.bind(this),
       destroy: this._destroy.bind(this),
@@ -44,10 +49,15 @@ class Scope extends Base {
     }
   }
 
-  _init (asyncId, type) {
+  _init (asyncId, type, triggerAsyncId, resource) {
     this._spans[asyncId] = this._active()
     this._types[asyncId] = type
 
+    if (hasKeepAliveBug && (type === 'TCPWRAP' || type === 'HTTPPARSER')) {
+      this._destroy(this._weaks.get(resource))
+      this._weaks.set(resource, asyncId)
+    }
+
     platform.metrics().increment('async.resources')
     platform.metrics().increment('async.resources.by.type', `resource_type:${type}`)
   }

src/scope/async_hooks/async_wrap.js

@@ -26,7 +26,7 @@ module.exports = {
 
     if (callbacks.init) {
       hooks.init = (uid, handle, provider, parentUid, parentHandle) => {
-        callbacks.init(uid, providers[provider])
+        callbacks.init(uid, providers[provider], parentUid, handle)
       }
     }
 

test/leak/scope/async_hooks.js

@@ -0,0 +1,109 @@
+'use strict'
+
+const tracer = require('../../..')
+
+const ASYNC_HOOKS = require('../../../ext/scopes').ASYNC_HOOKS
+
+tracer.init({
+  scope: ASYNC_HOOKS
+})
+
+const ah = require('async_hooks')
+const test = require('tape')
+const http = require('http')
+const getPort = require('get-port')
+const profile = require('../../profile')
+
+const host = '127.0.0.1'
+const listen = (port, hostname, listeningListener) => {
+  const server = http.createServer((req, res) => {
+    res.writeHead(200)
+    res.end()
+  })
+
+  return server.listen(port, hostname, listeningListener)
+}
+
+test('Scope should handle HTTP resource leaks in Node', t => {
+  getPort().then(port => {
+    const agent = new http.Agent({ keepAlive: true })
+    const listener = listen(port, host, () => {
+      profile(t, operation)
+        .then(() => {
+          listener.close()
+        })
+
+      function operation (done) {
+        const request = http.request({ host, port, agent }, res => {
+          res.resume()
+          done()
+        })
+
+        request.end()
+      }
+    })
+  })
+})
+
+test('Scope should not lose active span when handling leaks in Node', t => {
+  t.plan(1)
+
+  const asyncIds = new Set()
+  const leakIds = new Set()
+
+  let failed = 0
+
+  const hook = ah.createHook({
+    init (asyncId, type) {
+      asyncIds.add(asyncId)
+
+      if (type === 'TCPWRAP' || type === 'HTTPPARSER') {
+        leakIds.add(asyncId)
+      }
+    },
+    after (asyncId) {
+      if (leakIds.has(asyncId) && !asyncIds.has(asyncId)) {
+        failed++
+      }
+    },
+    destroy (asyncId) {
+      asyncIds.delete(asyncId)
+    }
+  })
+
+  hook.enable()
+
+  getPort().then(port => {
+    const agent = new http.Agent({
+      keepAlive: true,
+      maxSockets: 5,
+      maxFreeSockets: 5
+    })
+
+    const listener = listen(port, host, () => {
+      const promises = []
+
+      for (let i = 0; i < 100; i++) {
+        const promise = new Promise((resolve, reject) => {
+          http.get({ host, port, agent }, res => {
+            res.resume()
+            resolve()
+          })
+        })
+
+        promises.push(promise)
+      }
+
+      Promise.all(promises)
+        .then(() => {
+          if (failed) {
+            t.fail(`the active span was lost by ${failed} scopes`)
+          } else {
+            t.ok('no scope lost the active span')
+          }
+
+          listener.close()
+        })
+    })
+  })
+})

test/scope/async_hooks.spec.js

@@ -1,5 +1,6 @@
 'use strict'
 
+const semver = require('semver')
 const Scope = require('../../src/scope/async_hooks')
 const platform = require('../../src/platform')
 const testScope = require('./test')
@@ -50,5 +51,18 @@ describe('Scope', () => {
     expect(metrics.decrement).to.have.been.calledWith('async.resources.by.type')
   })
 
+  if (!semver.satisfies(process.version, '^8.13 || >=10.14.2')) {
+    it('should work around the HTTP keep-alive bug in Node', () => {
+      const resource = {}
+
+      sinon.spy(scope, '_destroy')
+
+      scope._init(1, 'TCPWRAP', 0, resource)
+      scope._init(1, 'TCPWRAP', 0, resource)
+
+      expect(scope._destroy).to.have.been.called
+    })
+  }
+
   testScope(() => scope)
 })