From 3ef0881985dd1400135f3ffd3105b23f3a5eb7d4 Mon Sep 17 00:00:00 2001
From: Taku Amano <taku@taaas.jp>
Date: Wed, 29 May 2024 14:35:55 +0900
Subject: [PATCH 1/4] test: add test for memory leak by AbortController

---
 package.json        |  2 +-
 test/server.test.ts | 72 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/package.json b/package.json
index 86aea27..77874dd 100644
--- a/package.json
+++ b/package.json
@@ -46,7 +46,7 @@
     }
   },
   "scripts": {
-    "test": "jest",
+    "test": "node --expose-gc ./node_modules/.bin/jest",
     "build": "tsup",
     "watch": "tsup --watch",
     "postbuild": "publint",
diff --git a/test/server.test.ts b/test/server.test.ts
index 9780fff..c7fad1c 100644
--- a/test/server.test.ts
+++ b/test/server.test.ts
@@ -8,7 +8,7 @@ import { compress } from 'hono/compress'
 import { poweredBy } from 'hono/powered-by'
 import { stream } from 'hono/streaming'
 import request from 'supertest'
-import { GlobalRequest, Request as LightweightRequest } from '../src/request'
+import { GlobalRequest, Request as LightweightRequest, getAbortController } from '../src/request'
 import { GlobalResponse, Response as LightweightResponse } from '../src/response'
 import { createAdaptorServer } from '../src/server'
 import type { HttpBindings } from '../src/types'
@@ -796,3 +796,73 @@ describe('overrideGlobalObjects', () => {
     })
   })
 })
+
+describe('Memory leak test', () => {
+  let counter = 0
+  const registry = new FinalizationRegistry(() => {
+    counter--
+  })
+  const app = new Hono()
+  const server = createAdaptorServer(app)
+
+  let onAbort: () => void
+  let reqReadyResolve: () => void
+  let reqReadyPromise: Promise<void>
+
+  app.use(async (c, next) => {
+    counter++
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    registry.register((c.req.raw as any)[getAbortController](), 'abortController')
+    await next()
+  })
+  app.get('/', (c) => c.text('Hello! Node!'))
+  app.post('/', async (c) => c.json(await c.req.json()))
+  app.get('/abort', async (c) => {
+    c.req.raw.signal.addEventListener('abort', () => onAbort())
+    reqReadyResolve?.()
+    await new Promise(() => {}) // never resolve
+  })
+
+  beforeEach(() => {
+    counter = 0
+    reqReadyPromise = new Promise<void>((r) => {
+      reqReadyResolve = r
+    })
+  })
+
+  afterAll(() => {
+    server.close()
+  })
+
+  it('Should not have memory leak - GET /', async () => {
+    await request(server).get('/')
+    global.gc?.()
+    await new Promise((resolve) => setTimeout(resolve, 10))
+    expect(counter).toBe(0)
+  })
+
+  it('Should not have memory leak - POST /', async () => {
+    await request(server).post('/').set('Content-Type', 'application/json').send({ foo: 'bar' })
+    global.gc?.()
+    await new Promise((resolve) => setTimeout(resolve, 10))
+    expect(counter).toBe(0)
+  })
+
+  it('Should not have memory leak - GET /abort', async () => {
+    const abortedPromise = new Promise<void>((resolve) => {
+      onAbort = resolve
+    })
+
+    const req = request(server)
+      .get('/abort')
+      .end(() => {})
+    await reqReadyPromise
+    req.abort()
+    await abortedPromise
+    await new Promise((resolve) => setTimeout(resolve, 10))
+
+    global.gc?.()
+    await new Promise((resolve) => setTimeout(resolve, 10))
+    expect(counter).toBe(0)
+  })
+})

From a02ec2105f960f34ffec0a79b4d7d4a4ad0875c1 Mon Sep 17 00:00:00 2001
From: Taku Amano <taku@taaas.jp>
Date: Wed, 29 May 2024 14:36:09 +0900
Subject: [PATCH 2/4] chore: update tsconfig.json for test/**/*.ts

---
 tsconfig.json | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tsconfig.json b/tsconfig.json
index 58f0cdd..c4e1276 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -13,10 +13,11 @@
       "jest",
       "node",
     ],
-    "rootDir": "./src",
+    "rootDir": ".",
     "outDir": "./dist",
   },
   "include": [
-    "src/**/*.ts"
+    "src/**/*.ts",
+    "test/**/*.ts"
   ],
 }
\ No newline at end of file

From d1f2645e41e84848bf12d463a342b09bc44c8f62 Mon Sep 17 00:00:00 2001
From: Taku Amano <taku@taaas.jp>
Date: Wed, 29 May 2024 21:47:40 +0900
Subject: [PATCH 3/4] refactor: check request error by `incoming.errored`
 instead of `incoming.destroyed`

---
 src/listener.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/listener.ts b/src/listener.ts
index 6022240..7967189 100644
--- a/src/listener.ts
+++ b/src/listener.ts
@@ -178,7 +178,7 @@ export const getRequestListener = (
 
       // Detect if request was aborted.
       outgoing.on('close', () => {
-        if (incoming.destroyed) {
+        if (incoming.errored) {
           req[getAbortController]().abort()
         }
       })

From a0be98b95ed5221d96c988565e5f5e6fa74228ee Mon Sep 17 00:00:00 2001
From: Taku Amano <taku@taaas.jp>
Date: Wed, 29 May 2024 21:48:14 +0900
Subject: [PATCH 4/4] refactor: add reason to abort controller by text

Due to the nodejs implementation, passing an object to `abort()` will cause a memory leak, so pass a string
---
 src/listener.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/listener.ts b/src/listener.ts
index 7967189..a9d46c2 100644
--- a/src/listener.ts
+++ b/src/listener.ts
@@ -179,7 +179,7 @@ export const getRequestListener = (
       // Detect if request was aborted.
       outgoing.on('close', () => {
         if (incoming.errored) {
-          req[getAbortController]().abort()
+          req[getAbortController]().abort(incoming.errored.toString())
         }
       })