Skip to content

Commit

Permalink
fix: move timers to use timeouts (#2923)
Browse files Browse the repository at this point in the history
timers now use setTimeout on a worker rather than setInterval to ensure we don't get overlaps
  • Loading branch information
gavinbarron authored Jan 3, 2024
1 parent 4d0db50 commit c9c4047
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 20 deletions.
42 changes: 24 additions & 18 deletions packages/mgt-chat/src/statefulClient/GraphNotificationClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ const isMembershipNotification = (o: Notification<Entity>): o is Notification<Aa

export class GraphNotificationClient {
private connection?: HubConnection = undefined;
private renewalInterval?: string;
private cleanupInterval?: string;
private renewalTimeout?: string;
private cleanupTimeout?: string;
private renewalCount = 0;
private chatId = '';
private sessionId = '';
Expand Down Expand Up @@ -88,8 +88,8 @@ export class GraphNotificationClient {
*/
public async tearDown() {
log('cleaning up graph notification resources');
if (this.cleanupInterval) this.timer.clearInterval(this.cleanupInterval);
if (this.renewalInterval) this.timer.clearInterval(this.renewalInterval);
if (this.cleanupTimeout) this.timer.clearTimeout(this.cleanupTimeout);
if (this.renewalTimeout) this.timer.clearTimeout(this.renewalTimeout);
this.timer.close();
await this.unsubscribeFromChatNotifications(this.chatId, this.sessionId);
}
Expand Down Expand Up @@ -179,8 +179,8 @@ export class GraphNotificationClient {

await this.subscriptionCache.cacheSubscription(this.chatId, this.sessionId, subscriptionRecord);

// only start timer once. -1 for renewalInterval is semaphore it has stopped.
if (this.renewalInterval === undefined) this.startRenewalTimer();
// only start timer once. undefined for renewalInterval is semaphore it has stopped.
if (this.renewalTimeout === undefined) this.startRenewalTimer();
};

private async subscribeToResource(resourcePath: string, changeTypes: ChangeTypes[]) {
Expand Down Expand Up @@ -218,20 +218,20 @@ export class GraphNotificationClient {
}

private readonly startRenewalTimer = () => {
if (this.renewalInterval !== undefined) this.timer.clearInterval(this.renewalInterval);
this.renewalInterval = this.timer.setInterval(this.syncTimerWrapper, appSettings.renewalTimerInterval * 1000);
log(`Start renewal timer . Id: ${this.renewalInterval}`);
if (this.renewalTimeout) this.timer.clearTimeout(this.renewalTimeout);
this.renewalTimeout = this.timer.setTimeout(this.syncRenewalTimerWrapper, appSettings.renewalTimerInterval * 1000);
log(`Start renewal timer . Id: ${this.renewalTimeout}`);
};

private readonly syncTimerWrapper = () => void this.renewalTimer();
private readonly syncRenewalTimerWrapper = () => void this.renewalTimer();

private readonly renewalTimer = async () => {
log(`running subscription renewal timer for chatId: ${this.chatId} sessionId: ${this.sessionId}`);
const subscriptions =
(await this.subscriptionCache.loadSubscriptions(this.chatId, this.sessionId))?.subscriptions || [];
if (subscriptions.length === 0) {
log(`No subscriptions found in session state. Stop renewal timer ${this.renewalInterval}.`);
clearInterval(this.renewalInterval);
log(`No subscriptions found in session state. Stop renewal timer ${this.renewalTimeout}.`);
if (this.renewalTimeout) this.timer.clearTimeout(this.renewalTimeout);
return;
}

Expand All @@ -245,18 +245,17 @@ export class GraphNotificationClient {
this.renewalCount++;
log(`Renewing Graph subscription. RenewalCount: ${this.renewalCount}`);
// stop interval to prevent new invokes until refresh is ready.
clearInterval(this.renewalInterval);
this.renewalInterval = undefined;
void this.renewChatSubscriptions();
if (this.renewalTimeout) this.timer.clearTimeout(this.renewalTimeout);
this.renewalTimeout = undefined;
await this.renewChatSubscriptions();
// There is one subscription that need expiration, all subscriptions will be renewed
break;
}
}
this.renewalTimeout = this.timer.setTimeout(this.syncRenewalTimerWrapper, appSettings.renewalTimerInterval * 1000);
};

public renewChatSubscriptions = async () => {
if (this.renewalInterval) this.timer.clearInterval(this.renewalInterval);

const expirationTime = new Date(
new Date().getTime() + appSettings.defaultSubscriptionLifetimeInMinutes * 60 * 1000
);
Expand All @@ -270,6 +269,12 @@ export class GraphNotificationClient {
log(`Invoked RenewSubscription ${subscription.id}`);
}
await Promise.all(awaits);
if (!this.renewalTimeout) {
this.renewalTimeout = this.timer.setTimeout(
this.syncRenewalTimerWrapper,
appSettings.renewalTimerInterval * 1000
);
}
};

public renewSubscription = async (subscriptionId: string, expirationDateTime: string): Promise<void> => {
Expand Down Expand Up @@ -325,7 +330,7 @@ export class GraphNotificationClient {
}

private startCleanupTimer() {
this.cleanupInterval = this.timer.setInterval(this.cleanupTimerSync, appSettings.removalTimerInterval * 1000);
this.cleanupTimeout = this.timer.setTimeout(this.cleanupTimerSync, appSettings.removalTimerInterval * 1000);
}

private readonly cleanupTimerSync = () => {
Expand All @@ -349,6 +354,7 @@ export class GraphNotificationClient {
for (const inactive of inactiveSubs) {
tasks.push(this.subscriptionCache.deleteCachedSubscriptions(inactive.chatId, inactive.sessionId));
}
this.startCleanupTimer();
};

public async closeSignalRConnection() {
Expand Down
25 changes: 25 additions & 0 deletions packages/mgt-chat/src/utils/Timer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,31 @@ export class Timer {
this.worker.port.onmessage = this.onMessage;
}

public setTimeout(callback: () => void, delay: number): string {
const timeoutWork: TimerWork = {
type: 'setTimeout',
id: uuid(),
delay
};

this.work.set(timeoutWork.id, callback);

this.worker.port.postMessage(timeoutWork);

return timeoutWork.id;
}

public clearTimeout(id: string): void {
if (this.work.has(id)) {
const timeoutWork: TimerWork = {
type: 'clearTimeout',
id
};
this.worker.port.postMessage(timeoutWork);
this.work.delete(id);
}
}

public setInterval(callback: () => void, delay: number): string {
const intervalWork: TimerWork = {
type: 'setInterval',
Expand Down
17 changes: 15 additions & 2 deletions packages/mgt-chat/src/utils/timerWorker.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export interface TimerWork {
id: string;
type: 'clearInterval' | 'setInterval' | 'runCallback';
type: 'clearInterval' | 'setInterval' | 'runCallback' | 'setTimeout' | 'clearTimeout';
delay?: number;
}

Expand All @@ -23,9 +23,22 @@ ctx.onconnect = (e: MessageEvent<unknown>) => {
intervals.set(jobId, interval);
break;
}
case 'clearInterval':
case 'setTimeout': {
const timeout = setTimeout(() => {
const message: TimerWork = { ...event.data, ...{ type: 'runCallback' } };
port.postMessage(message);
}, delay);
intervals.set(jobId, timeout);
break;
}
case 'clearTimeout': {
clearTimeout(intervals.get(jobId));
intervals.delete(jobId);
break;
}
case 'clearInterval':
clearInterval(intervals.get(jobId));
intervals.delete(jobId);
}
};
port.onmessage = handleMessage;
Expand Down

0 comments on commit c9c4047

Please sign in to comment.