From 8822630dd6ac28d6fe4e8c3b643756964aa57f1c Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Thu, 13 Jul 2023 19:41:04 +0200 Subject: [PATCH] Clean up promises after resolving (#52656) Long-hanging promises are retaining the closure context where it's being `await`'ed even it's resolved already. You can tell it from this screenshot: CleanShot 2023-07-13 at 18 09 11@2x In some cases, the entire `req` object is retained because of that. This increases the memory usage a bit. So this PR changes these promises to be cleaned up after they got resolved. Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/next/src/server/base-server.ts | 8 +++++++- .../default-route-matcher-manager.ts | 7 +++++-- packages/next/src/server/next.ts | 11 ++++++++--- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index d2eadc9c165de..ce31a771f3d85 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -1014,14 +1014,20 @@ export default abstract class Server { this.renderOpts.assetPrefix = prefix ? prefix.replace(/\/$/, '') : '' } + protected prepared: boolean = false protected preparedPromise: Promise | null = null /** * Runs async initialization of server. * It is idempotent, won't fire underlying initialization more than once. */ public async prepare(): Promise { + if (this.prepared) return + if (this.preparedPromise === null) { - this.preparedPromise = this.prepareImpl() + this.preparedPromise = this.prepareImpl().then(() => { + this.prepared = true + this.preparedPromise = null + }) } return this.preparedPromise } diff --git a/packages/next/src/server/future/route-matcher-managers/default-route-matcher-manager.ts b/packages/next/src/server/future/route-matcher-managers/default-route-matcher-manager.ts index 74d62871a04fe..6b47776376b6d 100644 --- a/packages/next/src/server/future/route-matcher-managers/default-route-matcher-manager.ts +++ b/packages/next/src/server/future/route-matcher-managers/default-route-matcher-manager.ts @@ -35,8 +35,11 @@ export class DefaultRouteMatcherManager implements RouteMatcherManager { } private waitTillReadyPromise?: Promise - public waitTillReady(): Promise { - return this.waitTillReadyPromise ?? Promise.resolve() + public async waitTillReady(): Promise { + if (this.waitTillReadyPromise) { + await this.waitTillReadyPromise + delete this.waitTillReadyPromise + } } private previousMatchers: ReadonlyArray = [] diff --git a/packages/next/src/server/next.ts b/packages/next/src/server/next.ts index 97023cd0349f7..fdf5509d511d1 100644 --- a/packages/next/src/server/next.ts +++ b/packages/next/src/server/next.ts @@ -51,6 +51,7 @@ const SYMBOL_LOAD_CONFIG = Symbol('next.load_config') export class NextServer { private serverPromise?: Promise private server?: Server + private reqHandler?: NodeRequestHandler private reqHandlerPromise?: Promise private preparedAssetPrefix?: string @@ -217,14 +218,18 @@ export class NextServer { } private async getServerRequestHandler() { + if (this.reqHandler) return this.reqHandler + // Memoize request handler creation if (!this.reqHandlerPromise) { - this.reqHandlerPromise = this.getServer().then((server) => - getTracer().wrap( + this.reqHandlerPromise = this.getServer().then((server) => { + this.reqHandler = getTracer().wrap( NextServerSpan.getServerRequestHandler, server.getRequestHandler().bind(server) ) - ) + delete this.reqHandlerPromise + return this.reqHandler + }) } return this.reqHandlerPromise }