From c32d8b155ff0395ccee10275b88dec935736e4ea Mon Sep 17 00:00:00 2001
From: Anmol Sethi <hi@nhooyr.io>
Date: Mon, 18 Jan 2021 08:30:00 -0500
Subject: [PATCH] heart.ts: Fix leak when server closes

This had me very confused for quite a while until I did a binary search
inspection on route/index.ts. Only with the heart.beat line commented
out did my tests pass without leaking.

They weren't leaking fds but just this heartbeat timer and node of
course prints just fds that are active when it detects some sort of leak
I guess and that made the whole thing very confusing. These fds are not
leaked and will close when node's event loop detects there are no more
callbacks to run.

no of handles 3

tcp stream {
  fd: 20,
  readable: false,
  writable: true,
  address: {},
  serverAddr: null
}

tcp stream {
  fd: 22,
  readable: false,
  writable: true,
  address: {},
  serverAddr: null
}

tcp stream {
  fd: 23,
  readable: true,
  writable: false,
  address: {},
  serverAddr: null
}

It kept printing the above text again and again for 60s and then the
test binary times out I think. I'm not sure if it was node printing the
stuff above or if it was a mocha thing. But it was really confusing...

cc @code-asher for thoughts on what was going on.

edit: It was the leaked-handles import in socket.test.ts!!!
Not sure if we should keep it, this was really confusing and misleading.
---
 src/node/heart.ts        | 9 +++++++++
 src/node/routes/index.ts | 3 +++
 test/integration.ts      | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/src/node/heart.ts b/src/node/heart.ts
index eed070e4e..1eada79b3 100644
--- a/src/node/heart.ts
+++ b/src/node/heart.ts
@@ -45,4 +45,13 @@ export class Heart {
         })
     }, this.heartbeatInterval)
   }
+
+  /**
+   * Call to clear any heartbeatTimer for shutdown.
+   */
+  public dispose(): void {
+    if (typeof this.heartbeatTimer !== "undefined") {
+      clearTimeout(this.heartbeatTimer)
+    }
+  }
 }
diff --git a/src/node/routes/index.ts b/src/node/routes/index.ts
index 4688d6a2e..006543679 100644
--- a/src/node/routes/index.ts
+++ b/src/node/routes/index.ts
@@ -55,6 +55,9 @@ export const register = async (
       })
     })
   })
+  server.on("close", () => {
+    heart.dispose()
+  })
 
   app.disable("x-powered-by")
   wsApp.disable("x-powered-by")
diff --git a/test/integration.ts b/test/integration.ts
index 6a321cae0..f829fe5de 100644
--- a/test/integration.ts
+++ b/test/integration.ts
@@ -8,6 +8,8 @@ export async function setup(
   argv: string[],
   configFile?: string,
 ): Promise<[express.Application, express.Application, httpserver.HttpServer, DefaultedArgs]> {
+  argv = ["--bind-addr=localhost:0", ...argv]
+
   const cliArgs = parse(argv)
   const configArgs = parseConfigFile(configFile || "", "test/integration.ts")
   const args = await setDefaults(cliArgs, configArgs)