Skip to content

Commit

Permalink
fix: don't memory-leak promises passed to waitUntil (#75041)
Browse files Browse the repository at this point in the history
### Overview

`next start` stores pending promises passed to `waitUntil` so that we
can await them before a (graceful) server shutdown and avoid
interrupting work that's still running.
The problem here was that this set lives a long time (the whole lifetime
of the server), but we weren't removing resolved promises from this set,
so we'd end up holding onto them forever and leaking memory.

This bug would be triggered by any code using `waitUntil` in `next
start` (notably, a recent change:
#74164)

### Details 
The bug here is that `AwaiterMulti` would hold onto resolved promises
indefinitely. `AwaiterMulti` ends up being used by
`NextNodeServer.getInternalWaitUntil`, which is what provides
`waitUntil` in `next start`.

The code was already attempting to clean up promises to avoid leaks. But
the problem was that we weren't adding `promise` to `this.promises`, we
were adding `promise.then(...)` which is a completely different object.
So the `this.promises.delete(promise)` that `cleanup` did was
effectively doing nothing, and `this.promises` would keep growing.
  • Loading branch information
lubieowoce committed Jan 20, 2025
1 parent 47102ca commit 20f5a79
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions packages/next/src/server/after/awaiter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,25 @@ export class AwaiterMulti {
}

public waitUntil = (promise: Promise<unknown>): void => {
// if a promise settles before we await it, we can drop it.
// if a promise settles before we await it, we should drop it --
// storing them indefinitely could result in a memory leak.
const cleanup = () => {
this.promises.delete(promise)
}

this.promises.add(
promise.then(cleanup, (err) => {
cleanup()
this.onError(err)
})
)
promise.then(cleanup, (err) => {
cleanup()
this.onError(err)
})

this.promises.add(promise)
}

public async awaiting(): Promise<void> {
while (this.promises.size > 0) {
const promises = Array.from(this.promises)
this.promises.clear()
await Promise.all(promises)
await Promise.allSettled(promises)
}
}
}
Expand Down

0 comments on commit 20f5a79

Please sign in to comment.