From 96a9115ee7baa80d46e4349025ef852532d7a847 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 11 Jun 2026 12:21:07 +0200 Subject: [PATCH] Fix a minor resource leak with display names and avatars I noticed that calls to createDisplayNameBehavior$ and createAvatarUrlBehavior$ were technically leaking resources since they reused the ObservableScope from their outer scope, which in practice lasts for the entire lifetime of the CallViewModel. This would not have had any noticeable effect unless you had other participants leave and rejoin the same call many thousands of times. --- src/state/CallViewModel/CallViewModel.ts | 14 ++-- .../MatrixMemberMetadata.test.ts | 69 ++++++++++++------- .../remoteMembers/MatrixMemberMetadata.ts | 14 ++-- 3 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index aaf679505..83b7cc618 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -766,11 +766,13 @@ export function createCallViewModel$( pretendToBeDisconnected$: localMembership.reconnecting$, displayName$: scope.behavior( matrixMemberMetadataStore - .createDisplayNameBehavior$(userId) + .createDisplayNameBehavior$(scope, userId) .pipe(map((name) => name ?? userId)), ), - mxcAvatarUrl$: - matrixMemberMetadataStore.createAvatarUrlBehavior$(userId), + mxcAvatarUrl$: matrixMemberMetadataStore.createAvatarUrlBehavior$( + scope, + userId, + ), handRaised$: scope.behavior( handsRaised$.pipe(map((v) => v[mediaId]?.time ?? null)), ), @@ -811,8 +813,10 @@ export function createCallViewModel$( map((members) => members.get(userId)?.rawDisplayName || userId), ), ), - mxcAvatarUrl$: - matrixMemberMetadataStore.createAvatarUrlBehavior$(userId), + mxcAvatarUrl$: matrixMemberMetadataStore.createAvatarUrlBehavior$( + scope, + userId, + ), pickupState$, muteStates, }), diff --git a/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.test.ts b/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.test.ts index f7dd775c8..79026e211 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.test.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.test.ts @@ -105,7 +105,7 @@ describe("MatrixMemberMetadata", () => { } it("should show our own user if present in rtc session and room", () => { - withTestScheduler(({ behavior, expectObservable }) => { + withTestScheduler(({ scope, behavior, expectObservable }) => { fakeMemberWith({ userId: "@local:example.com", rawDisplayName: "it's a me", @@ -118,8 +118,10 @@ describe("MatrixMemberMetadata", () => { memberships$, createRoomMembers$(testScope, mockMatrixRoom), ); - const dn$ = - metadataStore.createDisplayNameBehavior$("@local:example.com"); + const dn$ = metadataStore.createDisplayNameBehavior$( + scope, + "@local:example.com", + ); expectObservable(dn$).toBe("a", { a: "it's a me", @@ -146,7 +148,7 @@ describe("MatrixMemberMetadata", () => { it("should get displayName for users", () => { setUpBasicRoom(); - withTestScheduler(({ behavior, expectObservable }) => { + withTestScheduler(({ scope, behavior, expectObservable }) => { const memberships$ = behavior("a", { a: [ mockRtcMembership("@alice:example.com", "DEVICE1"), @@ -158,8 +160,10 @@ describe("MatrixMemberMetadata", () => { memberships$, createRoomMembers$(testScope, mockMatrixRoom), ); - const aliceDispName$ = - metadataStore.createDisplayNameBehavior$("@alice:example.com"); + const aliceDispName$ = metadataStore.createDisplayNameBehavior$( + scope, + "@alice:example.com", + ); expectObservable(aliceDispName$).toBe("a", { a: "Alice", @@ -322,7 +326,7 @@ describe("MatrixMemberMetadata", () => { }); it("should track individual member id with createDisplayNameBehavior", () => { - withTestScheduler(({ behavior, schedule, expectObservable }) => { + withTestScheduler(({ scope, behavior, schedule, expectObservable }) => { setUpBasicRoom(); const BOB = "@bob:example.com"; const CARL = "@carl:example.com"; @@ -356,8 +360,8 @@ describe("MatrixMemberMetadata", () => { createRoomMembers$(testScope, mockMatrixRoom), ); - const bob$ = metadataStore.createDisplayNameBehavior$(BOB); - const carl$ = metadataStore.createDisplayNameBehavior$(CARL); + const bob$ = metadataStore.createDisplayNameBehavior$(scope, BOB); + const carl$ = metadataStore.createDisplayNameBehavior$(scope, CARL); expectObservable(bob$).toBe("abc-", { a: undefined, @@ -378,7 +382,7 @@ describe("MatrixMemberMetadata", () => { }); it("should disambiguate users with invisible characters", () => { - withTestScheduler(({ behavior, expectObservable }) => { + withTestScheduler(({ scope, behavior, expectObservable }) => { const bobRtcMember = mockRtcMembership("@bob:example.org", "BBBB"); const bobZeroWidthSpaceRtcMember = mockRtcMembership( "@bob2:example.org", @@ -411,12 +415,18 @@ describe("MatrixMemberMetadata", () => { createRoomMembers$(testScope, mockMatrixRoom), ); - const bob$ = - metadataStore.createDisplayNameBehavior$("@bob:example.org"); - const bob2$ = - metadataStore.createDisplayNameBehavior$("@bob2:example.org"); - const carol$ = - metadataStore.createDisplayNameBehavior$("@carol:example.org"); + const bob$ = metadataStore.createDisplayNameBehavior$( + scope, + "@bob:example.org", + ); + const bob2$ = metadataStore.createDisplayNameBehavior$( + scope, + "@bob2:example.org", + ); + const carol$ = metadataStore.createDisplayNameBehavior$( + scope, + "@carol:example.org", + ); expectObservable(bob$).toBe("ab", { a: "Bob", b: "Bob (@bob:example.org)", @@ -517,7 +527,7 @@ describe("MatrixMemberMetadata", () => { } it("should use avatar url from room members", () => { - withTestScheduler(({ behavior, expectObservable }) => { + withTestScheduler(({ scope, behavior, expectObservable }) => { fakeMemberWith({ userId: "@local:example.com", }); @@ -536,11 +546,15 @@ describe("MatrixMemberMetadata", () => { memberships$, createRoomMembers$(testScope, mockMatrixRoom), ); - const local$ = - metadataStore.createAvatarUrlBehavior$("@local:example.com"); + const local$ = metadataStore.createAvatarUrlBehavior$( + scope, + "@local:example.com", + ); - const alice$ = - metadataStore.createAvatarUrlBehavior$("@alice:example.com"); + const alice$ = metadataStore.createAvatarUrlBehavior$( + scope, + "@alice:example.com", + ); expectObservable(local$).toBe("a", { a: "mxc://example.com/@local:example.com", @@ -558,7 +572,7 @@ describe("MatrixMemberMetadata", () => { }); it("should update on avatar change and user join/leave", () => { - withTestScheduler(({ behavior, schedule, expectObservable }) => { + withTestScheduler(({ scope, behavior, schedule, expectObservable }) => { fakeMemberWith({ userId: "@carl:example.com" }); fakeMemberWith({ userId: "@bob:example.com" }); const memberships$ = behavior("ab-d", { @@ -585,9 +599,14 @@ describe("MatrixMemberMetadata", () => { }, }); - const bob$ = metadataStore.createAvatarUrlBehavior$("@bob:example.com"); - const carl$ = - metadataStore.createAvatarUrlBehavior$("@carl:example.com"); + const bob$ = metadataStore.createAvatarUrlBehavior$( + scope, + "@bob:example.com", + ); + const carl$ = metadataStore.createAvatarUrlBehavior$( + scope, + "@carl:example.com", + ); expectObservable(bob$).toBe("a---", { a: "mxc://example.com/@bob:example.com", }); diff --git a/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.ts b/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.ts index d9be2d350..19c6fdcd5 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixMemberMetadata.ts @@ -115,8 +115,14 @@ export const createMatrixMemberMetadata$ = ( memberships$: Behavior[]>, roomMembers$: Behavior, ): { - createDisplayNameBehavior$: (userId: string) => Behavior; - createAvatarUrlBehavior$: (userId: string) => Behavior; + createDisplayNameBehavior$: ( + scope: ObservableScope, + userId: string, + ) => Behavior; + createAvatarUrlBehavior$: ( + scope: ObservableScope, + userId: string, + ) => Behavior; displaynameMap$: Behavior>; avatarMap$: Behavior>; } => { @@ -136,13 +142,13 @@ export const createMatrixMemberMetadata$ = ( ), ); return { - createDisplayNameBehavior$: (userId: string) => + createDisplayNameBehavior$: (scope: ObservableScope, userId: string) => scope.behavior( displaynameMap$.pipe( map((displaynameMap) => displaynameMap.get(userId)), ), ), - createAvatarUrlBehavior$: (userId: string) => + createAvatarUrlBehavior$: (scope: ObservableScope, userId: string) => scope.behavior( roomMembers$.pipe( map((roomMembers) => roomMembers.get(userId)?.getMxcAvatarUrl()),