Dispose HTTP server

Shares code with the test HTTP server.  For now it is a function but
maybe we should make it a class that is extended by tests.

Author: code-asher

src/common/emitter.ts

@@ -7,7 +7,7 @@ import { logger } from "@coder/logger"
 export type Callback<T, R = void | Promise<void>> = (t: T, p: Promise<void>) => R
 
 export interface Disposable {
-  dispose(): void
+  dispose(): void | Promise<void>
 }
 
 export interface Event<T> {

src/node/app.ts

@@ -4,13 +4,24 @@ import express, { Express } from "express"
 import { promises as fs } from "fs"
 import http from "http"
 import * as httpolyglot from "httpolyglot"
+import { Disposable } from "../common/emitter"
 import * as util from "../common/util"
 import { DefaultedArgs } from "./cli"
+import { disposer } from "./http"
 import { isNodeJSErrnoException } from "./util"
 import { handleUpgrade } from "./wsRouter"
 
 type ListenOptions = Pick<DefaultedArgs, "socket" | "port" | "host">
 
+export interface App extends Disposable {
+  /** Handles regular HTTP requests. */
+  router: Express
+  /** Handles websocket requests. */
+  wsRouter: Express
+  /** The underlying HTTP server. */
+  server: http.Server
+}
+
 const listen = (server: http.Server, { host, port, socket }: ListenOptions) => {
   return new Promise<void>(async (resolve, reject) => {
     server.on("error", reject)
@@ -41,27 +52,28 @@ const listen = (server: http.Server, { host, port, socket }: ListenOptions) => {
 /**
  * Create an Express app and an HTTP/S server to serve it.
  */
-export const createApp = async (args: DefaultedArgs): Promise<[Express, Express, http.Server]> => {
-  const app = express()
-
-  app.use(compression())
+export const createApp = async (args: DefaultedArgs): Promise<App> => {
+  const router = express()
+  router.use(compression())
 
   const server = args.cert
     ? httpolyglot.createServer(
         {
           cert: args.cert && (await fs.readFile(args.cert.value)),
           key: args["cert-key"] && (await fs.readFile(args["cert-key"])),
         },
-        app,
+        router,
       )
-    : http.createServer(app)
+    : http.createServer(router)
+
+  const dispose = disposer(server)
 
   await listen(server, args)
 
-  const wsApp = express()
-  handleUpgrade(wsApp, server)
+  const wsRouter = express()
+  handleUpgrade(wsRouter, server)
 
-  return [app, wsApp, server]
+  return { router, wsRouter, server, dispose }
 }
 
 /**

src/node/http.ts

@@ -1,8 +1,11 @@
 import { field, logger } from "@coder/logger"
 import * as express from "express"
 import * as expressCore from "express-serve-static-core"
+import * as http from "http"
+import * as net from "net"
 import path from "path"
 import qs from "qs"
+import { Disposable } from "../common/emitter"
 import { HttpCode, HttpError } from "../common/http"
 import { normalize } from "../common/util"
 import { AuthType, DefaultedArgs } from "./cli"
@@ -188,3 +191,53 @@ export const getCookieDomain = (host: string, proxyDomains: string[]): string |
   logger.debug("got cookie doman", field("host", host))
   return host || undefined
 }
+
+/**
+ * Return a function capable of fully disposing an HTTP server.
+ */
+export function disposer(server: http.Server): Disposable["dispose"] {
+  const sockets = new Set<net.Socket>()
+  let cleanupTimeout: undefined | NodeJS.Timeout
+
+  server.on("connection", (socket) => {
+    sockets.add(socket)
+
+    socket.on("close", () => {
+      sockets.delete(socket)
+
+      if (cleanupTimeout && sockets.size === 0) {
+        clearTimeout(cleanupTimeout)
+        cleanupTimeout = undefined
+      }
+    })
+  })
+
+  return () => {
+    return new Promise<void>((resolve, reject) => {
+      // The whole reason we need this disposer is because close will not
+      // actually close anything; it only prevents future connections then waits
+      // until everything is closed.
+      server.close((err) => {
+        if (err) {
+          return reject(err)
+        }
+
+        resolve()
+      })
+
+      // If there are sockets remaining we might need to force close them or
+      // this promise might never resolve.
+      if (sockets.size > 0) {
+        // Give sockets a chance to close up shop.
+        cleanupTimeout = setTimeout(() => {
+          cleanupTimeout = undefined
+
+          for (const socket of sockets.values()) {
+            console.warn("a socket was left hanging")
+            socket.destroy()
+          }
+        }, 1000)
+      }
+    })
+  }
+}

src/node/main.ts

@@ -121,9 +121,9 @@ export const runCodeServer = async (
     )
   }
 
-  const [app, wsApp, server] = await createApp(args)
-  const serverAddress = ensureAddress(server, args.cert ? "https" : "http")
-  const disposeApp = await register(app, wsApp, server, args)
+  const app = await createApp(args)
+  const serverAddress = ensureAddress(app.server, args.cert ? "https" : "http")
+  const disposeRoutes = await register(app, args)
 
   logger.info(`Using config file ${humanPath(args.config)}`)
   logger.info(`HTTP server listening on ${serverAddress.toString()} ${args.link ? "(randomized by --link)" : ""}`)
@@ -194,10 +194,11 @@ export const runCodeServer = async (
   }
 
   return {
-    server,
-    dispose: () => {
-      disposeApp()
+    server: app.server,
+    dispose: async () => {
       linkAgent?.kill()
+      disposeRoutes()
+      await app.dispose()
     },
   }
 }

src/node/routes/index.ts

@@ -2,13 +2,13 @@ import { logger } from "@coder/logger"
 import cookieParser from "cookie-parser"
 import * as express from "express"
 import { promises as fs } from "fs"
-import http from "http"
 import * as path from "path"
 import * as tls from "tls"
 import * as pluginapi from "../../../typings/pluginapi"
 import { Disposable } from "../../common/emitter"
 import { HttpCode, HttpError } from "../../common/http"
 import { plural } from "../../common/util"
+import { App } from "../app"
 import { AuthType, DefaultedArgs } from "../cli"
 import { commit, isDevMode, rootPath } from "../constants"
 import { Heart } from "../heart"
@@ -28,15 +28,10 @@ import { createVSServerRouter, VSServerResult } from "./vscode"
 /**
  * Register all routes and middleware.
  */
-export const register = async (
-  app: express.Express,
-  wsApp: express.Express,
-  server: http.Server,
-  args: DefaultedArgs,
-): Promise<Disposable["dispose"]> => {
+export const register = async (app: App, args: DefaultedArgs): Promise<Disposable["dispose"]> => {
   const heart = new Heart(path.join(paths.data, "heartbeat"), async () => {
     return new Promise((resolve, reject) => {
-      server.getConnections((error, count) => {
+      app.server.getConnections((error, count) => {
         if (error) {
           return reject(error)
         }
@@ -46,11 +41,11 @@ export const register = async (
     })
   })
 
-  app.disable("x-powered-by")
-  wsApp.disable("x-powered-by")
+  app.router.disable("x-powered-by")
+  app.wsRouter.disable("x-powered-by")
 
-  app.use(cookieParser())
-  wsApp.use(cookieParser())
+  app.router.use(cookieParser())
+  app.wsRouter.use(cookieParser())
 
   const common: express.RequestHandler = (req, _, next) => {
     // /healthz|/healthz/ needs to be excluded otherwise health checks will make
@@ -66,10 +61,10 @@ export const register = async (
     next()
   }
 
-  app.use(common)
-  wsApp.use(common)
+  app.router.use(common)
+  app.wsRouter.use(common)
 
-  app.use(async (req, res, next) => {
+  app.router.use(async (req, res, next) => {
     // If we're handling TLS ensure all requests are redirected to HTTPS.
     // TODO: This does *NOT* work if you have a base path since to specify the
     // protocol we need to specify the whole path.
@@ -87,24 +82,24 @@ export const register = async (
     next()
   })
 
-  app.use("/", domainProxy.router)
-  wsApp.use("/", domainProxy.wsRouter.router)
+  app.router.use("/", domainProxy.router)
+  app.wsRouter.use("/", domainProxy.wsRouter.router)
 
-  app.all("/proxy/(:port)(/*)?", (req, res) => {
+  app.router.all("/proxy/(:port)(/*)?", (req, res) => {
     pathProxy.proxy(req, res)
   })
-  wsApp.get("/proxy/(:port)(/*)?", async (req) => {
+  app.wsRouter.get("/proxy/(:port)(/*)?", async (req) => {
     await pathProxy.wsProxy(req as pluginapi.WebsocketRequest)
   })
   // These two routes pass through the path directly.
   // So the proxied app must be aware it is running
   // under /absproxy/<someport>/
-  app.all("/absproxy/(:port)(/*)?", (req, res) => {
+  app.router.all("/absproxy/(:port)(/*)?", (req, res) => {
     pathProxy.proxy(req, res, {
       passthroughPath: true,
     })
   })
-  wsApp.get("/absproxy/(:port)(/*)?", async (req) => {
+  app.wsRouter.get("/absproxy/(:port)(/*)?", async (req) => {
     await pathProxy.wsProxy(req as pluginapi.WebsocketRequest, {
       passthroughPath: true,
     })
@@ -115,40 +110,40 @@ export const register = async (
     const workingDir = args._ && args._.length > 0 ? path.resolve(args._[args._.length - 1]) : undefined
     pluginApi = new PluginAPI(logger, process.env.CS_PLUGIN, process.env.CS_PLUGIN_PATH, workingDir)
     await pluginApi.loadPlugins()
-    pluginApi.mount(app, wsApp)
-    app.use("/api/applications", ensureAuthenticated, apps.router(pluginApi))
+    pluginApi.mount(app.router, app.wsRouter)
+    app.router.use("/api/applications", ensureAuthenticated, apps.router(pluginApi))
   }
 
-  app.use(express.json())
-  app.use(express.urlencoded({ extended: true }))
+  app.router.use(express.json())
+  app.router.use(express.urlencoded({ extended: true }))
 
-  app.use(
+  app.router.use(
     "/_static",
     express.static(rootPath, {
       cacheControl: commit !== "development",
     }),
   )
 
-  app.use("/healthz", health.router)
-  wsApp.use("/healthz", health.wsRouter.router)
+  app.router.use("/healthz", health.router)
+  app.wsRouter.use("/healthz", health.wsRouter.router)
 
   if (args.auth === AuthType.Password) {
-    app.use("/login", login.router)
-    app.use("/logout", logout.router)
+    app.router.use("/login", login.router)
+    app.router.use("/logout", logout.router)
   } else {
-    app.all("/login", (req, res) => redirect(req, res, "/", {}))
-    app.all("/logout", (req, res) => redirect(req, res, "/", {}))
+    app.router.all("/login", (req, res) => redirect(req, res, "/", {}))
+    app.router.all("/logout", (req, res) => redirect(req, res, "/", {}))
   }
 
-  app.use("/update", update.router)
+  app.router.use("/update", update.router)
 
   let vscode: VSServerResult
   try {
     vscode = await createVSServerRouter(args)
-    app.use("/", vscode.router)
-    wsApp.use("/", vscode.wsRouter.router)
-    app.use("/vscode", vscode.router)
-    wsApp.use("/vscode", vscode.wsRouter.router)
+    app.router.use("/", vscode.router)
+    app.wsRouter.use("/", vscode.wsRouter.router)
+    app.router.use("/vscode", vscode.router)
+    app.wsRouter.use("/vscode", vscode.wsRouter.router)
   } catch (error: any) {
     if (isDevMode) {
       logger.warn(error)
@@ -158,12 +153,12 @@ export const register = async (
     }
   }
 
-  app.use(() => {
+  app.router.use(() => {
     throw new HttpError("Not Found", HttpCode.NotFound)
   })
 
-  app.use(errorHandler)
-  wsApp.use(wsErrorHandler)
+  app.router.use(errorHandler)
+  app.wsRouter.use(wsErrorHandler)
 
   return () => {
     heart.dispose()