From 589f090d0bdd6c6b0c1e1bcd8de7507037d4322e Mon Sep 17 00:00:00 2001
From: Jimmy Lai <laijimmy0@gmail.com>
Date: Thu, 8 Dec 2022 20:50:46 +0100
Subject: [PATCH] memory: fix 2 memory leaks in `next-dev` (#43859)

This PR fixes two memory leaks I found debugging with @sokra.

## 1) Leak in `next-server.ts`

The first leak was caused by the fact that the `require.cache` associated to the `next-server` module was not being cleared up properly, so we leaked context from modules required in that page, like API routes.

## 2)  Leak with React Fetch

When evaluating a route, we also evaluated the `react.shared-subset.development.js` module where React patches the `fetch` function. The problem is that when re-evaluating a route as part of hot reloading, we were patching over the previously patched `fetch` function.
The result of this operation meant that we were keeping a reference to the context of the previous `fetch` and thus to the previous route context, thus creating a memory leak, since we only needed the new context.

## Test plan

Checked manually the heap snapshots of a test app.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
---
 .../plugins/nextjs-require-cache-hot-reloader.ts  |  2 ++
 packages/next/server/dev/next-dev-server.ts       | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts b/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts
index d661c1f3f379e5d..1cd9b47d1df0ff7 100644
--- a/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts
+++ b/packages/next/build/webpack/plugins/nextjs-require-cache-hot-reloader.ts
@@ -10,6 +10,8 @@ type WebpackPluginInstance = webpack.WebpackPluginInstance
 const originModules = [
   require.resolve('../../../server/require'),
   require.resolve('../../../server/load-components'),
+  require.resolve('../../../server/next-server'),
+  require.resolve('../../../compiled/react-server-dom-webpack/client'),
 ]
 
 const RUNTIME_NAMES = ['webpack-runtime', 'webpack-api-runtime']
diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts
index c4dae0ce636173f..3b793ba777e35bc 100644
--- a/packages/next/server/dev/next-dev-server.ts
+++ b/packages/next/server/dev/next-dev-server.ts
@@ -109,6 +109,7 @@ export default class DevServer extends Server {
   private edgeFunctions?: RoutingItem[]
   private verifyingTypeScript?: boolean
   private usingTypeScript?: boolean
+  private originalFetch?: typeof fetch
 
   protected staticPathsWorker?: { [key: string]: any } & {
     loadStaticPaths: typeof import('./static-paths-worker').loadStaticPaths
@@ -149,6 +150,7 @@ export default class DevServer extends Server {
 
   constructor(options: Options) {
     super({ ...options, dev: true })
+    this.persistPatchedGlobals()
     this.renderOpts.dev = true
     ;(this.renderOpts as any).ErrorDebug = ReactDevOverlay
     this.devReady = new Promise((resolve) => {
@@ -1385,6 +1387,14 @@ export default class DevServer extends Server {
     return this.hotReloader!.ensurePage({ page: pathname, clientOnly: false })
   }
 
+  private persistPatchedGlobals(): void {
+    this.originalFetch = global.fetch
+  }
+
+  private restorePatchedGlobals(): void {
+    global.fetch = this.originalFetch!
+  }
+
   protected async findPageComponents({
     pathname,
     query,
@@ -1418,6 +1428,11 @@ export default class DevServer extends Server {
         this.serverCSSManifest = super.getServerCSSManifest()
       }
       this.fontLoaderManifest = super.getFontLoaderManifest()
+      // before we re-evaluate a route module, we want to restore globals that might
+      // have been patched previously to their original state so that we don't
+      // patch on top of the previous patch, which would keep the context of the previous
+      // patched global in memory, creating a memory leak.
+      this.restorePatchedGlobals()
 
       return super.findPageComponents({ pathname, query, params, isAppPath })
     } catch (err) {