Skip to content

Commit 20b3fb6

Browse files
committed
fix(core): add timeout to BackgroundProcessManager.close()
Race close() against a 10s timeout so it always resolves, even when jobs ignore the onTerminate signal (e.g. subscription processor). On timeout, logs a warning with pending job descriptions and clears the jobs set to prevent zombie jobs persisting across close/open cycles. Resolves stuck "Stopping" state in DataStore after network drops. Related: #13035 #10612 #12359
1 parent 24c0a0b commit 20b3fb6

File tree

3 files changed

+60
-0
lines changed

3 files changed

+60
-0
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@aws-amplify/core': patch
3+
---
4+
5+
fix(core): add timeout to BackgroundProcessManager.close() to prevent permanent stuck state

packages/core/__tests__/BackgroundProcessManager.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,4 +664,31 @@ describe('BackgroundProcessManager', () => {
664664
unblock?.();
665665
await close;
666666
});
667+
668+
test('close() resolves after timeout even if jobs never complete', async () => {
669+
const manager = new BackgroundProcessManager();
670+
// eslint-disable-next-line @typescript-eslint/no-empty-function
671+
const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
672+
673+
manager.add(
674+
// eslint-disable-next-line @typescript-eslint/no-empty-function
675+
() => new Promise<void>(() => {}),
676+
'hanging job',
677+
);
678+
679+
const start = Date.now();
680+
await manager.close();
681+
const elapsed = Date.now() - start;
682+
683+
expect(manager.isClosed).toBe(true);
684+
expect(manager.length).toBe(0);
685+
// should resolve around the 10s internal timeout, not hang forever
686+
expect(elapsed).toBeLessThan(15_000);
687+
expect(warnSpy).toHaveBeenCalledWith(
688+
expect.stringContaining('timed out'),
689+
expect.arrayContaining(['hanging job']),
690+
);
691+
692+
warnSpy.mockRestore();
693+
}, 20_000);
667694
});

packages/core/src/BackgroundProcessManager/BackgroundProcessManager.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,35 @@ export class BackgroundProcessManager {
389389
Array.from(this.jobs).map(j => j.promise),
390390
);
391391

392+
const CLOSE_TIMEOUT_MS = 10_000;
393+
let timer: ReturnType<typeof setTimeout>;
394+
let didTimeout = false;
395+
const timeoutPromise = new Promise<PromiseSettledResult<any>[]>(
396+
resolve => {
397+
timer = setTimeout(() => {
398+
didTimeout = true;
399+
resolve([]);
400+
}, CLOSE_TIMEOUT_MS);
401+
},
402+
);
403+
this._closingPromise = Promise.race([
404+
this._closingPromise,
405+
timeoutPromise,
406+
]);
392407
await this._closingPromise;
408+
clearTimeout(timer!);
409+
410+
if (didTimeout) {
411+
const pendingDescriptions = this.pending.filter(Boolean);
412+
413+
// eslint-disable-next-line no-console
414+
console.warn(
415+
`BackgroundProcessManager.close() timed out after ${CLOSE_TIMEOUT_MS}ms with ${this.jobs.size} pending job(s):`,
416+
pendingDescriptions,
417+
);
418+
this.jobs.clear();
419+
}
420+
393421
this._state = BackgroundProcessManagerState.Closed;
394422
}
395423

0 commit comments

Comments
 (0)