Skip to content

Commit afd016c

Browse files
committed
fix: enhance timeout handling for sub-second values and improve edge case tests
1 parent 6247d52 commit afd016c

File tree

2 files changed

+26
-13
lines changed

2 files changed

+26
-13
lines changed

src/timeout-wheel.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ export const addTimeout = (
5757
): void => {
5858
removeTimeout(key);
5959

60-
// Fallback to setTimeout if wheel size is exceeded or ms is not divisible by SECOND
61-
if (ms > MAX_WHEEL_MS || ms % SECOND !== 0) {
60+
// Fallback to setTimeout if wheel size is exceeded, ms is sub-second, or ms is not divisible by SECOND
61+
if (ms < SECOND || ms > MAX_WHEEL_MS || ms % SECOND !== 0) {
6262
keyMap.set(key, [setTimeout(handleCallback.bind(null, [key, cb]), ms)]); // Store timeout ID instead of slot
6363

6464
return;
@@ -74,8 +74,15 @@ export const addTimeout = (
7474
if (!timer) {
7575
timer = setInterval(() => {
7676
position = (position + 1) % WHEEL_SIZE;
77-
wheel[position].forEach(handleCallback);
78-
wheel[position] = [];
77+
const slot = wheel[position];
78+
79+
// Use slot.length directly (not cached) so mid-iteration mutations
80+
// from callbacks (e.g. removeTimeout) are handled correctly
81+
for (let i = 0; i < slot.length; i++) {
82+
handleCallback(slot[i]);
83+
}
84+
85+
slot.length = 0; // Reuse array, avoid GC allocation
7986

8087
if (!keyMap.size && timer) {
8188
clearInterval(timer);
@@ -93,10 +100,12 @@ export const removeTimeout = (key: string): void => {
93100
if (Array.isArray(slotOrTimeout)) {
94101
clearTimeout(slotOrTimeout[0]);
95102
} else {
96-
wheel[slotOrTimeout].splice(
97-
wheel[slotOrTimeout].findIndex(([k]) => k === key),
98-
1,
99-
);
103+
const slotArr = wheel[slotOrTimeout];
104+
const idx = slotArr.findIndex(([k]) => k === key);
105+
106+
if (idx !== -1) {
107+
slotArr.splice(idx, 1);
108+
}
100109
}
101110

102111
keyMap.delete(key);
@@ -110,18 +119,22 @@ export const removeTimeout = (key: string): void => {
110119

111120
export const clearAllTimeouts = () => {
112121
// Clear native setTimeout timeouts first!
113-
keyMap.forEach((value) => {
122+
for (const value of keyMap.values()) {
114123
if (Array.isArray(value)) {
115124
clearTimeout(value[0]);
116125
}
117-
});
126+
}
118127

119128
if (timer) {
120129
clearInterval(timer);
121130
timer = null;
122131
}
123132

124133
keyMap.clear();
125-
wheel.forEach((slot) => (slot.length = 0));
134+
135+
for (let i = 0; i < WHEEL_SIZE; i++) {
136+
wheel[i].length = 0;
137+
}
138+
126139
position = 0;
127140
};

test/timeout-wheel.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,13 +265,13 @@ describe('Timeout Wheel', () => {
265265
});
266266

267267
describe('Edge Cases', () => {
268-
it('should handle zero timeout', () => {
268+
it('should handle zero timeout via setTimeout fallback', () => {
269269
const callback = jest.fn();
270270

271271
addTimeout('zero', callback, 0);
272272

273273
jest.advanceTimersByTime(1);
274-
expect(callback).toHaveBeenCalledTimes(0); // Should not execute immediately
274+
expect(callback).toHaveBeenCalledTimes(1); // Fires via setTimeout(cb, 0)
275275
});
276276

277277
it('should handle timeout at wheel boundary', () => {

0 commit comments

Comments
 (0)