Skip to content

Commit

Permalink
Clean up promises after resolving (#52656)
Browse files Browse the repository at this point in the history
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:

<img width="1822" alt="CleanShot 2023-07-13 at 18 09 11@2x"
src="https://github.com/vercel/next.js/assets/3676859/1d6210b0-16ba-440e-a8b6-c8c4343f4850">

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>
  • Loading branch information
shuding and kodiakhq[bot] committed Jul 13, 2023
1 parent ca1129c commit 8822630
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
8 changes: 7 additions & 1 deletion packages/next/src/server/base-server.ts
Expand Up @@ -1014,14 +1014,20 @@ export default abstract class Server<ServerOptions extends Options = Options> {
this.renderOpts.assetPrefix = prefix ? prefix.replace(/\/$/, '') : ''
}

protected prepared: boolean = false
protected preparedPromise: Promise<void> | null = null
/**
* Runs async initialization of server.
* It is idempotent, won't fire underlying initialization more than once.
*/
public async prepare(): Promise<void> {
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
}
Expand Down
Expand Up @@ -35,8 +35,11 @@ export class DefaultRouteMatcherManager implements RouteMatcherManager {
}

private waitTillReadyPromise?: Promise<void>
public waitTillReady(): Promise<void> {
return this.waitTillReadyPromise ?? Promise.resolve()
public async waitTillReady(): Promise<void> {
if (this.waitTillReadyPromise) {
await this.waitTillReadyPromise
delete this.waitTillReadyPromise
}
}

private previousMatchers: ReadonlyArray<RouteMatcher> = []
Expand Down
11 changes: 8 additions & 3 deletions packages/next/src/server/next.ts
Expand Up @@ -51,6 +51,7 @@ const SYMBOL_LOAD_CONFIG = Symbol('next.load_config')
export class NextServer {
private serverPromise?: Promise<Server>
private server?: Server
private reqHandler?: NodeRequestHandler
private reqHandlerPromise?: Promise<NodeRequestHandler>
private preparedAssetPrefix?: string

Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 8822630

Please sign in to comment.