Skip to content

Commit e1d31ef

Browse files
authored
fix: replace setInterval with recursive setTimeout in Master.ts to pr… (#2869)
If this PR fixes an issue, link it below. If not, delete these two lines. Resolves #2868 ## Description: This PR addresses a critical memory leak in the Master server process (causing ~30GB RAM usage). The issue was caused by `setInterval` calling `fetchLobbies()` every 100ms. When `fetchLobbies` took longer than 100ms to complete (due to network latency or load), requests would pile up indefinitely, creating a massive queue of pending Promises and open sockets. I have refactored the polling logic into a generic `startPolling` utility (in `src/server/PollingLoop.ts`) that uses a recursive `setTimeout` pattern. This ensures that the next `fetchLobbies` call is only scheduled *after* the previous one has completed (successfully or failed), preventing any request pile-up. ## Please complete the following: - [x] I have added screenshots for all UI updates (N/A - backend only) - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file (N/A - no user facing text) - [x] I have added relevant tests to the test directory (`tests/PollingLoop.test.ts`) - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: codimo
1 parent 0421c4e commit e1d31ef

File tree

5 files changed

+163
-69
lines changed

5 files changed

+163
-69
lines changed

src/server/Master.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { GameInfo } from "../core/Schemas";
1212
import { generateID } from "../core/Util";
1313
import { logger } from "./Logger";
1414
import { MapPlaylist } from "./MapPlaylist";
15+
import { startPolling } from "./PollingLoop";
1516
import { renderHtml } from "./RenderHtml";
1617

1718
const config = getServerConfigFromServer();
@@ -176,15 +177,12 @@ export async function startMaster() {
176177
});
177178
};
178179

179-
setInterval(
180-
() =>
181-
fetchLobbies().then((lobbies) => {
182-
if (lobbies === 0) {
183-
scheduleLobbies();
184-
}
185-
}),
186-
100,
187-
);
180+
startPolling(async () => {
181+
const lobbies = await fetchLobbies();
182+
if (lobbies === 0) {
183+
scheduleLobbies();
184+
}
185+
}, 100);
188186
}
189187
}
190188
});

src/server/PollingLoop.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { logger } from "./Logger";
2+
3+
const log = logger.child({ comp: "polling" });
4+
5+
/**
6+
* Starts a polling loop that executes the given async task effectively recursively using setTimeout.
7+
* This guarantees that the next execution only starts after the previous one has completed (or failed),
8+
* preventing request pile-ups.
9+
*
10+
* @param task The async function to execute.
11+
* @param intervalMs The delay in milliseconds before the next execution.
12+
*/
13+
export function startPolling(task: () => Promise<void>, intervalMs: number) {
14+
const runLoop = () => {
15+
task()
16+
.catch((error) => {
17+
log.error("Error in polling loop:", error);
18+
})
19+
.finally(() => {
20+
setTimeout(runLoop, intervalMs);
21+
});
22+
};
23+
runLoop();
24+
}

src/server/PrivilegeRefresher.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { base64url } from "jose";
22
import { Logger } from "winston";
33
import { CosmeticsSchema } from "../core/CosmeticSchemas";
4+
import { startPolling } from "./PollingLoop";
45
import {
56
FailOpenPrivilegeChecker,
67
PrivilegeChecker,
@@ -28,12 +29,7 @@ export class PrivilegeRefresher {
2829
this.log.info(
2930
`Starting privilege refresher with interval ${this.refreshInterval}`,
3031
);
31-
// Add some jitter to the initial load and the interval.
32-
setTimeout(() => this.loadPrivilegeChecker(), Math.random() * 1000);
33-
setInterval(
34-
() => this.loadPrivilegeChecker(),
35-
this.refreshInterval + Math.random() * 1000,
36-
);
32+
startPolling(() => this.loadPrivilegeChecker(), this.refreshInterval);
3733
}
3834

3935
public get(): PrivilegeChecker {

src/server/Worker.ts

Lines changed: 53 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { logger } from "./Logger";
2727

2828
import { GameEnv } from "../core/configuration/Config";
2929
import { MapPlaylist } from "./MapPlaylist";
30+
import { startPolling } from "./PollingLoop";
3031
import { PrivilegeRefresher } from "./PrivilegeRefresher";
3132
import { verifyTurnstileToken } from "./Turnstile";
3233
import { initWorkerMetrics } from "./WorkerMetrics";
@@ -43,7 +44,7 @@ export async function startWorker() {
4344

4445
setTimeout(
4546
() => {
46-
pollLobby(gm);
47+
startMatchmakingPolling(gm);
4748
},
4849
1000 + Math.random() * 2000,
4950
);
@@ -483,63 +484,61 @@ export async function startWorker() {
483484
});
484485
}
485486

486-
async function pollLobby(gm: GameManager) {
487-
try {
488-
const url = `${config.jwtIssuer() + "/matchmaking/checkin"}`;
489-
const gameId = generateGameIdForWorker();
490-
if (gameId === null) {
491-
log.warn(`Failed to generate game ID for worker ${workerId}`);
492-
return;
493-
}
487+
async function startMatchmakingPolling(gm: GameManager) {
488+
startPolling(
489+
async () => {
490+
try {
491+
const url = `${config.jwtIssuer() + "/matchmaking/checkin"}`;
492+
const gameId = generateGameIdForWorker();
493+
if (gameId === null) {
494+
log.warn(`Failed to generate game ID for worker ${workerId}`);
495+
return;
496+
}
494497

495-
const controller = new AbortController();
496-
const timeoutId = setTimeout(() => controller.abort(), 20000);
497-
const response = await fetch(url, {
498-
method: "POST",
499-
headers: {
500-
"Content-Type": "application/json",
501-
"x-api-key": config.apiKey(),
502-
},
503-
body: JSON.stringify({
504-
id: workerId,
505-
gameId: gameId,
506-
ccu: gm.activeClients(),
507-
instanceId: process.env.INSTANCE_ID,
508-
}),
509-
signal: controller.signal,
510-
});
498+
const controller = new AbortController();
499+
const timeoutId = setTimeout(() => controller.abort(), 20000);
500+
const response = await fetch(url, {
501+
method: "POST",
502+
headers: {
503+
"Content-Type": "application/json",
504+
"x-api-key": config.apiKey(),
505+
},
506+
body: JSON.stringify({
507+
id: workerId,
508+
gameId: gameId,
509+
ccu: gm.activeClients(),
510+
instanceId: process.env.INSTANCE_ID,
511+
}),
512+
signal: controller.signal,
513+
});
511514

512-
clearTimeout(timeoutId);
515+
clearTimeout(timeoutId);
513516

514-
if (!response.ok) {
515-
log.warn(
516-
`Failed to poll lobby: ${response.status} ${response.statusText}`,
517-
);
518-
return;
519-
}
517+
if (!response.ok) {
518+
log.warn(
519+
`Failed to poll lobby: ${response.status} ${response.statusText}`,
520+
);
521+
return;
522+
}
520523

521-
const data = await response.json();
522-
log.info(`Lobby poll successful:`, data);
523-
524-
if (data.assignment) {
525-
const gameConfig = playlist.get1v1Config();
526-
const game = gm.createGame(gameId, gameConfig);
527-
setTimeout(() => {
528-
// Wait a few seconds to allow clients to connect.
529-
console.log(`Starting game ${gameId}`);
530-
game.start();
531-
}, 5000);
532-
}
533-
} catch (error) {
534-
log.error(`Error polling lobby:`, error);
535-
} finally {
536-
setTimeout(
537-
() => {
538-
pollLobby(gm);
539-
},
540-
5000 + Math.random() * 1000,
541-
);
542-
}
524+
const data = await response.json();
525+
log.info(`Lobby poll successful:`, data);
526+
527+
if (data.assignment) {
528+
const gameConfig = playlist.get1v1Config();
529+
const game = gm.createGame(gameId, gameConfig);
530+
setTimeout(() => {
531+
// Wait a few seconds to allow clients to connect.
532+
console.log(`Starting game ${gameId}`);
533+
game.start();
534+
}, 5000);
535+
}
536+
} catch (error) {
537+
log.error(`Error polling lobby:`, error);
538+
}
539+
},
540+
5000 + Math.random() * 1000,
541+
);
543542
}
544543

545544
// TODO: This is a hack to generate a game ID for the worker.

tests/server/PollingLoop.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import { startPolling } from "../../src/server/PollingLoop";
3+
4+
vi.mock("../../src/server/Logger", () => ({
5+
logger: {
6+
child: () => ({
7+
error: vi.fn(),
8+
}),
9+
},
10+
}));
11+
12+
describe("PollingLoop", () => {
13+
beforeEach(() => {
14+
vi.useFakeTimers();
15+
});
16+
17+
afterEach(() => {
18+
vi.restoreAllMocks();
19+
});
20+
21+
it("should not start the next task until the previous one completes", async () => {
22+
let taskCallCount = 0;
23+
let resolveTask: ((value?: void) => void) | undefined;
24+
25+
const task = vi.fn().mockImplementation(() => {
26+
taskCallCount++;
27+
return new Promise<void>((resolve) => {
28+
resolveTask = resolve;
29+
});
30+
});
31+
32+
startPolling(task, 100);
33+
34+
// Initial call
35+
expect(taskCallCount).toBe(1);
36+
37+
// Advance time past the interval - should NOT trigger next call yet
38+
await vi.advanceTimersByTimeAsync(200);
39+
expect(taskCallCount).toBe(1);
40+
41+
// Resolve the first task
42+
if (resolveTask) resolveTask();
43+
44+
// Wait for microtasks (promise callbacks, finally block) to run
45+
await new Promise(process.nextTick);
46+
47+
// NOW advance time to trigger the scheduled continuation
48+
await vi.advanceTimersByTimeAsync(100);
49+
50+
expect(taskCallCount).toBe(2);
51+
});
52+
53+
it("should continue polling even if a task fails", async () => {
54+
let taskCallCount = 0;
55+
const task = vi.fn().mockImplementation(async () => {
56+
taskCallCount++;
57+
if (taskCallCount === 1) {
58+
throw new Error("Task failed");
59+
}
60+
});
61+
62+
startPolling(task, 100);
63+
64+
// First call
65+
expect(taskCallCount).toBe(1);
66+
67+
// Wait for rejection and finally block
68+
await new Promise(process.nextTick);
69+
await new Promise(process.nextTick);
70+
71+
// Advance time
72+
await vi.advanceTimersByTimeAsync(100);
73+
74+
// Second call
75+
expect(taskCallCount).toBe(2);
76+
});
77+
});

0 commit comments

Comments
 (0)