Skip to content

Commit

Permalink
Modify stopSync to block if sync is currently active (#889)
Browse files Browse the repository at this point in the history
- Add a missing changset for #887:

`stopSync` now blocks if a current sync is in progress before clearing the interval. An optional timeout can be defined, the default is 2 seconds. After this timeout it will throw.

TestHarness has been updated to stop sync before clearing storage, previously this caused an issue where an ongoing sync would attempt to sign messages for DID that no longer had keys after clearing storage.

#890 has been created to better address this by creating a signal to gracefully stop sync immediately.
  • Loading branch information
LiranCohen authored Sep 6, 2024
1 parent da3630a commit e578e20
Show file tree
Hide file tree
Showing 10 changed files with 310 additions and 21 deletions.
8 changes: 8 additions & 0 deletions .changeset/afraid-geese-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@web5/agent": patch
"@web5/identity-agent": patch
"@web5/proxy-agent": patch
"@web5/user-agent": patch
---

Fix sync race condition issue
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"@changesets/cli": "^2.27.5",
"@npmcli/package-json": "5.0.0",
"@typescript-eslint/eslint-plugin": "7.9.0",
"@web5/dwn-server": "0.4.8",
"@web5/dwn-server": "0.4.9",
"audit-ci": "^7.0.1",
"eslint-plugin-mocha": "10.4.3",
"globals": "^13.24.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/agent/src/sync-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class AgentSyncApi implements SyncEngine {
return this._syncEngine.startSync(params);
}

public stopSync(): void {
this._syncEngine.stopSync();
public stopSync(timeout?: number): Promise<void> {
return this._syncEngine.stopSync(timeout);
}
}
17 changes: 16 additions & 1 deletion packages/agent/src/sync-engine-level.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,22 @@ export class SyncEngineLevel implements SyncEngine {
}
}

public stopSync(): void {
/**
* stopSync currently awaits the completion of the current sync operation before stopping the sync interval.
* TODO: implement a signal to gracefully stop sync immediately https://github.com/TBD54566975/web5-js/issues/890
*/
public async stopSync(timeout: number = 2000): Promise<void> {
let elapsedTimeout = 0;

while(this._syncLock) {
if (elapsedTimeout >= timeout) {
throw new Error(`SyncEngineLevel: Existing sync operation did not complete within ${timeout} milliseconds.`);
}

elapsedTimeout += 100;
await new Promise((resolve) => setTimeout(resolve, timeout < 100 ? timeout : 100));
}

if (this._syncIntervalId) {
clearInterval(this._syncIntervalId);
this._syncIntervalId = undefined;
Expand Down
3 changes: 3 additions & 0 deletions packages/agent/src/test-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ export class PlatformAgentTestHarness {
}

public async clearStorage(): Promise<void> {
// first stop any ongoing sync operations
await this.agent.sync.stopSync();

// @ts-expect-error since normally this property shouldn't be set to undefined.
this.agent.agentDid = undefined;
await this.didResolverCache.clear();
Expand Down
5 changes: 4 additions & 1 deletion packages/agent/src/types/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export interface SyncEngine {
startSync(params: { interval: string }): Promise<void>;
/**
* Stops the periodic sync operation, will complete the current sync operation if one is already in progress.
*
* @param timeout the maximum amount of time, in milliseconds, to wait for the current sync operation to complete. Default is 2000 (2 seconds).
* @throws {Error} if the sync operation fails to stop before the timeout.
*/
stopSync(): void;
stopSync(timeout?: number): Promise<void>;
}
Loading

0 comments on commit e578e20

Please sign in to comment.