Skip to content

Commit c6b7f26

Browse files
committed
Fix memory leak in retry loop caused by AbortSignal.any()
Replace `AbortSignal.any()` with `linkSignals` from `@ydbjs/abortable@^6.1.0`, which properly cleans up event listeners using `Symbol.dispose`. Also fix signal handling in `setTimeout` and correct the order of error/attempt updates in the catch block. Signed-off-by: Vladislav Polyakov <polRk@ydb.tech>
1 parent c6b7c31 commit c6b7f26

File tree

4 files changed

+26
-11
lines changed

4 files changed

+26
-11
lines changed

.changeset/pswbdeilqd.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@ydbjs/retry': minor
3+
---
4+
5+
Fix memory leak in retry loop caused by `AbortSignal.any()`
6+
7+
`AbortSignal.any()` registers event listeners on all source signals but never removes them, leading to a listener leak on every retry attempt. Replace it with `linkSignals` from `@ydbjs/abortable@^6.1.0`, which uses `Symbol.dispose` to clean up listeners immediately after each attempt via `using`.
8+
9+
Additional correctness fixes in the same loop:
10+
11+
- Fix order of `ctx.error` / `ctx.attempt` updates so error is recorded before incrementing attempt counter
12+
- Rename local `retry` variable to `willRetry` to avoid shadowing the outer function name
13+
- Pass the composed `signal` (instead of raw `cfg.signal`) to `setTimeout` so abort is properly respected during the inter-attempt delay
14+
- Fix oxlint directive from `disable` to `disable-next-line` to suppress only the intended line

package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/retry/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
"attw": "attw --pack --profile esm-only"
4949
},
5050
"dependencies": {
51-
"@ydbjs/abortable": "^6.0.0",
51+
"@ydbjs/abortable": "^6.1.0",
5252
"@ydbjs/api": "^6.0.0",
5353
"@ydbjs/debug": "^6.0.0",
5454
"@ydbjs/error": "^6.0.0",

packages/retry/src/index.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { setTimeout } from 'timers/promises'
22

3-
import { abortable } from '@ydbjs/abortable'
3+
import { abortable, linkSignals } from '@ydbjs/abortable'
44
import { StatusIds_StatusCode } from '@ydbjs/api/operation'
55
import { loggers } from '@ydbjs/debug'
66
import { CommitError, YDBError } from '@ydbjs/error'
@@ -34,20 +34,21 @@ export async function retry<R>(
3434
(budget = typeof config.budget === 'number' ? config.budget : config.budget!(ctx, config))
3535
) {
3636
let ac = new AbortController()
37-
let signal = cfg.signal ? AbortSignal.any([cfg.signal, ac.signal]) : ac.signal
37+
using linkedSignal = linkSignals(cfg.signal, ac.signal)
3838

3939
let start = Date.now()
40+
let signal = linkedSignal.signal
4041

4142
try {
4243
signal.throwIfAborted()
4344
dbg.log('attempt %d: calling retry function', ctx.attempt + 1)
44-
// oxlint-disable no-await-in-loop
45+
// oxlint-disable-next-line no-await-in-loop
4546
let result = await abortable(signal, Promise.resolve(fn(signal)))
4647
dbg.log('attempt %d: success', ctx.attempt + 1)
4748
return result
4849
} catch (error) {
49-
ctx.attempt += 1
5050
ctx.error = error
51+
ctx.attempt += 1
5152

5253
if (error instanceof Error && error.name === 'AbortError') {
5354
dbg.log('attempt %d: abort error, not retryable', ctx.attempt)
@@ -59,14 +60,14 @@ export async function retry<R>(
5960
throw error
6061
}
6162

62-
let retry: boolean
63+
let willRetry: boolean
6364
if (typeof config.retry === 'boolean') {
64-
retry = config.retry
65+
willRetry = config.retry
6566
} else {
66-
retry = config.retry?.(ctx.error, cfg.idempotent ?? false) ?? false
67+
willRetry = config.retry?.(ctx.error, cfg.idempotent ?? false) ?? false
6768
}
6869

69-
if (!retry || ctx.attempt >= budget) {
70+
if (!willRetry || ctx.attempt >= budget) {
7071
dbg.log('attempt %d: not retrying, error: %O', ctx.attempt, error)
7172
break
7273
}
@@ -86,7 +87,7 @@ export async function retry<R>(
8687

8788
dbg.log('attempt %d: waiting %d ms before next retry', ctx.attempt, remaining)
8889
// oxlint-disable no-await-in-loop
89-
await setTimeout(remaining, void 0, { signal: cfg.signal })
90+
await setTimeout(remaining, void 0, { signal })
9091

9192
if (config.onRetry) {
9293
config.onRetry(ctx)

0 commit comments

Comments
 (0)