Skip to content

Commit 972503f

Browse files
authored
Merge pull request #2376 from Mentra-Community/hotfix/session-timer-leaks-v2
hotfix: fix 5 timer leaks causing OOM crashes (France 1465MB RSS with 28 sessions)
2 parents eb97614 + f1e5a14 commit 972503f

File tree

5 files changed

+61
-2
lines changed

5 files changed

+61
-2
lines changed

cloud/packages/cloud/src/services/session/AppAudioStreamManager.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ export class AppAudioStreamManager {
151151
/** This user's active streams (typically 0 or 1) */
152152
private streams = new Map<string, ActiveStream>();
153153

154+
/** Timers for deferred stream cleanup — tracked so dispose() can cancel them */
155+
private pendingCleanupTimers = new Set<NodeJS.Timeout>();
156+
154157
private disposed = false;
155158

156159
constructor(userId: string, logger: Logger, sendPlayRequest: SendPlayRequestFn) {
@@ -286,7 +289,11 @@ export class AppAudioStreamManager {
286289
writer.close().catch(() => {});
287290
// Clean up the stream entry after a short delay to let the HTTP
288291
// response drain
289-
setTimeout(() => this.streams.delete(streamId), 1000);
292+
const timer = setTimeout(() => {
293+
this.pendingCleanupTimers.delete(timer);
294+
this.streams.delete(streamId);
295+
}, 1000);
296+
this.pendingCleanupTimers.add(timer);
290297
} else {
291298
// Start the abandon timeout (resets on each write from the SDK)
292299
this.resetAbandonTimer(streamId, stream);
@@ -351,7 +358,11 @@ export class AppAudioStreamManager {
351358
// Already closed
352359
}
353360
// Clean up after a short delay to let HTTP response drain
354-
setTimeout(() => this.streams.delete(streamId), 1000);
361+
const timer = setTimeout(() => {
362+
this.pendingCleanupTimers.delete(timer);
363+
this.streams.delete(streamId);
364+
}, 1000);
365+
this.pendingCleanupTimers.add(timer);
355366
} else if (stream.pendingChunks.length > 0) {
356367
// There's buffered audio but no phone reader. The next claimStream()
357368
// call will flush it and then close. If the phone never reconnects,
@@ -414,6 +425,12 @@ export class AppAudioStreamManager {
414425
this.destroyStream(streamId);
415426
}
416427

428+
// Clear any pending cleanup timers so their closures don't prevent GC
429+
for (const timer of this.pendingCleanupTimers) {
430+
clearTimeout(timer);
431+
}
432+
this.pendingCleanupTimers.clear();
433+
417434
this.logger.debug("AppAudioStreamManager disposed");
418435
}
419436

cloud/packages/cloud/src/services/session/AppManager.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,13 @@ export class AppManager {
661661
return new Promise<AppStartResult>((resolve) => {
662662
// Set up a listener for when the existing attempt completes
663663
const checkCompletion = () => {
664+
// Guard: if the session was disposed while we were polling,
665+
// resolve immediately to avoid holding a reference to the dead session.
666+
if (this.disposed) {
667+
resolve({ success: false, error: { stage: "CONNECTION", message: "Session disposed while waiting" } });
668+
return;
669+
}
670+
664671
if (!this.pendingConnections.has(packageName)) {
665672
// Existing attempt completed, check final state
666673
if (this.userSession.runningApps.has(packageName)) {

cloud/packages/cloud/src/services/session/UserSession.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import CalendarManager from "./CalendarManager";
3333
import { DashboardManager } from "./dashboard";
3434
import DeviceManager from "./DeviceManager";
3535
import { handleAppMessage as appMessageHandler, handleGlassesMessage as glassesMessageHandler } from "./handlers";
36+
import { clearSubscriptionChangeTimer } from "./handlers/app-message-handler";
3637
import LocationManager from "./LocationManager";
3738
import MicrophoneManager from "./MicrophoneManager";
3839
import PhotoManager from "./PhotoManager";
@@ -439,6 +440,7 @@ export class UserSession {
439440
if (this.microphoneManager) {
440441
this.logger.info(`[UserSession:updateWebSocket] Scheduling mic state resync after WebSocket reconnect`);
441442
setTimeout(() => {
443+
if (this.disposed) return;
442444
if (this.microphoneManager && this.websocket?.readyState === WebSocketReadyState.OPEN) {
443445
this.logger.info(`[UserSession:updateWebSocket] Forcing mic state resync after WebSocket reconnect`);
444446
this.microphoneManager.forceResync();
@@ -773,6 +775,17 @@ export class UserSession {
773775

774776
this.logger.warn(`[UserSession:dispose]: Disposing UserSession: ${this.userId}`);
775777

778+
// Clear the subscription-change debounce timer for this user. Its closure
779+
// captures `userSession`, so leaving it alive would prevent GC of the
780+
// disposed UserSession until the timer fires.
781+
// Only clear if this is still the active session for this userId — during
782+
// reconnect races, a stale session's dispose must not cancel the newer
783+
// session's pending subscription-change timer. The timer map is keyed by
784+
// userId, so a blind clear would affect whichever session currently owns it.
785+
if (UserSession.sessions.get(this.userId) === this || !UserSession.sessions.has(this.userId)) {
786+
clearSubscriptionChangeTimer(this.userId);
787+
}
788+
776789
// Clean up all tracked resources (removes event listeners, clears timers)
777790
// This must happen BEFORE disposing managers to prevent stale callbacks
778791
this.resources.dispose();

cloud/packages/cloud/src/services/session/handlers/app-message-handler.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,21 @@ export enum AppErrorCode {
6363
const subscriptionChangeTimers = new Map<string, NodeJS.Timeout>();
6464
const SUBSCRIPTION_DEBOUNCE_MS = 500;
6565

66+
/**
67+
* Clears any pending subscription-change debounce timer for the given user.
68+
*
69+
* Must be called from UserSession.dispose() to prevent the timer's closure
70+
* from holding a strong reference to the disposed UserSession, which would
71+
* keep the entire session object graph alive until the debounce fires.
72+
*/
73+
export function clearSubscriptionChangeTimer(userId: string): void {
74+
const timer = subscriptionChangeTimers.get(userId);
75+
if (timer) {
76+
clearTimeout(timer);
77+
subscriptionChangeTimers.delete(userId);
78+
}
79+
}
80+
6681
/**
6782
* Handle incoming app message by routing to appropriate managers
6883
*

cloud/packages/cloud/src/services/session/translation/providers/SonioxTranslationProvider.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,13 @@ class SonioxTranslationStream implements TranslationStreamInstance {
156156
}
157157

158158
private async connect(): Promise<void> {
159+
// Silently return instead of rejecting — closeHandler schedules
160+
// setTimeout(() => this.connect(), ...) without .catch(), so rejecting
161+
// here would create an unhandled rejection during normal shutdown.
162+
if (this.disposed || this.isClosing) {
163+
return;
164+
}
165+
159166
return new Promise((resolve, reject) => {
160167
try {
161168
// Create WebSocket connection

0 commit comments

Comments
 (0)