Skip to content

Commit c2d25d9

Browse files
authored
Fix possible unknown state (With reproducible test) (#4944)
Signed-off-by: Timo K <[email protected]>
1 parent c4e1e07 commit c2d25d9

File tree

2 files changed

+41
-2
lines changed

2 files changed

+41
-2
lines changed

spec/unit/matrixrtc/MembershipManager.spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,32 @@ describe("MembershipManager", () => {
511511

512512
expect(client._unstable_updateDelayedEvent).toHaveBeenCalled();
513513
});
514+
515+
it("updates the UpdateExpiry entry in the action scheduler", async () => {
516+
const manager = new MembershipManager({}, room, client, () => undefined);
517+
manager.join([focus], focusActive);
518+
await jest.advanceTimersByTimeAsync(1);
519+
// clearing all mocks before checking what happens when calling: `onRTCSessionMemberUpdate`
520+
(client.sendStateEvent as Mock).mockClear();
521+
(client._unstable_updateDelayedEvent as Mock).mockClear();
522+
(client._unstable_sendDelayedStateEvent as Mock).mockClear();
523+
524+
(client._unstable_updateDelayedEvent as Mock<any>).mockRejectedValueOnce(
525+
new MatrixError({ errcode: "M_NOT_FOUND" }),
526+
);
527+
528+
const { resolve } = createAsyncHandle(client._unstable_sendDelayedStateEvent);
529+
await jest.advanceTimersByTimeAsync(10_000);
530+
await manager.onRTCSessionMemberUpdate([mockCallMembership(membershipTemplate, room.roomId)]);
531+
resolve({ delay_id: "id" });
532+
await jest.advanceTimersByTimeAsync(10_000);
533+
534+
expect(client.sendStateEvent).toHaveBeenCalled();
535+
expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalled();
536+
537+
expect(client._unstable_updateDelayedEvent).toHaveBeenCalled();
538+
expect(manager.status).toBe(Status.Connected);
539+
});
514540
});
515541

516542
// TODO: Not sure about this name

src/matrixrtc/MembershipManager.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ export class MembershipManager
243243
this.logger.warn("Missing own membership: force re-join");
244244
this.state.hasMemberStateEvent = false;
245245

246-
if (this.scheduler.actions.find((a) => sendingMembershipActions.includes(a.type as MembershipActionType))) {
246+
if (this.scheduler.actions.some((a) => sendingMembershipActions.includes(a.type as MembershipActionType))) {
247247
this.logger.error(
248248
"tried adding another `SendDelayedEvent` actions even though we already have one in the Queue\nActionQueueOnMemberUpdate:",
249249
this.scheduler.actions,
@@ -624,8 +624,21 @@ export class MembershipManager
624624
this.state.expireUpdateIterations = 1;
625625
this.state.hasMemberStateEvent = true;
626626
this.resetRateLimitCounter(MembershipActionType.SendJoinEvent);
627+
// An UpdateExpiry action might be left over from a previous join event.
628+
// We can reach sendJoinEvent when the delayed leave event gets send by the HS.
629+
// The branch where we might have a leftover UpdateExpiry action is:
630+
// RestartDelayedEvent (cannot find it, server removed it)
631+
// -> SendDelayedEvent (send new delayed event)
632+
// -> SendJoinEvent (here with a still scheduled UpdateExpiry action)
633+
const actionsWithoutUpdateExpiry = this.scheduler.actions.filter(
634+
(a) =>
635+
a.type !== MembershipActionType.UpdateExpiry && // A new UpdateExpiry action with an updated will be scheduled,
636+
a.type !== MembershipActionType.SendJoinEvent, // Manually remove the SendJoinEvent action,
637+
);
627638
return {
628-
insert: [
639+
replace: [
640+
...actionsWithoutUpdateExpiry,
641+
// To check if the delayed event is still there or got removed by inserting the stateEvent, we need to restart it.
629642
{ ts: Date.now(), type: MembershipActionType.RestartDelayedEvent },
630643
{
631644
ts: this.computeNextExpiryActionTs(this.state.expireUpdateIterations),

0 commit comments

Comments
 (0)