Socket recycling causes memory leaks

[timdp]

If sockets are recycled, in our case because we're using agentkeepalive as the agent, got's listeners don't always get removed from the socket after it's done its thing. We validated this by instrumenting the connect listener to store a unique ID on every new request ...

// At the top
let count = 0

// In requestAsEventEmitter()
req.connection.on('connect', () => {
  req.connection.__id = req.connection.__id || ++count
  console.log('connect', req.connection.__id)

... and then firing a few requests in sequence using the same socket:

const got = require('got')
const Agent = require('agentkeepalive')

const agent = new Agent({timeout: 100, maxSockets: 1})

;(async () => {
  for (let i = 0; i < 3; i++) {
    await got('http://localhost:31337', {agent})
  }
})()

For the first request, the socket connection listener is hit once; for the second, twice; for the third, thrice; etc. So clearly, something isn't getting cleaned up.

Of course, creating lots of internal state that isn't garbage-collected creates quite a big issue. So unless we're missing something, perhaps it should be made clear that recycling sockets isn't supported for the time being. In the longer term, got should probably clean up after itself even if the socket isn't disposed.

This wasn't the case with version 6, so presumably, since version 7, one of the closures in requestAsEventEmitter() is leaky. In general, personally, I'm a fan of using .once() where possible, and calling .removeEventListener() explicitly whenever I can. I feel like that would also be the more prudent approach here.

This was originally reported by @pietermees as part of his investigation for #353. The EventEmitter leak warning there actually wasn't a red herring at all.

Thanks!