self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2024-11-24 08:12:24 +02:00
Code Issues Releases Activity

All: Improved sync locks so that they do not prevent upgrading a sync target

Browse Source
This commit is contained in:
Laurent Cozic 2021-08-18 15:49:43 +01:00
parent 1efe3d3c6a
commit 06ed58b809
4 changed files with 68 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/lib/Synchronizer.ts
View File

@ -443,7 +443,7 @@ export default class Synchronizer {
const previousE2EE = localInfo.e2ee;
logger.info('Sync target info differs between local and remote - merging infos: ', newInfo.toObject());
await this.lockHandler().acquireLock(LockType.Exclusive, this.appType_, this.clientId_);
await this.lockHandler().acquireLock(LockType.Exclusive, this.appType_, this.clientId_, { clearExistingSyncLocksFromTheSameClient: true });
await uploadSyncInfo(this.api(), newInfo);
await saveLocalSyncInfo(newInfo);
await this.lockHandler().releaseLock(LockType.Exclusive, this.appType_, this.clientId_);

53
packages/lib/services/synchronizer/LockHandler.ts
View File

@ -18,6 +18,29 @@ export interface Lock {
updatedTime?: number;
}
export interface AcquireLockOptions {
// In theory, a client that tries to acquire an exclusive lock shouldn't
// also have a sync lock. It can however happen when the app is closed
// before the end of the sync process, and then the user tries to upgrade
// the sync target.
//
// So maybe we could always automatically clear the sync locks (that belongs
// to the current client) when acquiring an exclusive lock, but to be safe
// we make the behaviour explicit via this option. It is used for example
// when migrating a sync target.
//
// https://discourse.joplinapp.org/t/error-upgrading-to-2-3-3/19549/4?u=laurent
clearExistingSyncLocksFromTheSameClient?: boolean;
timeoutMs?: number;
}
function defaultAcquireLockOptions(): AcquireLockOptions {
return {
clearExistingSyncLocksFromTheSameClient: false,
timeoutMs: 0,
};
}
interface RefreshTimer {
id: any;
inProgress: boolean;
@ -198,7 +221,7 @@ export default class LockHandler {
return `(${lock.clientType} #${lock.clientId})`;
}
private async acquireExclusiveLock(clientType: string, clientId: string, timeoutMs: number = 0): Promise<Lock> {
private async acquireExclusiveLock(clientType: string, clientId: string, options: AcquireLockOptions = null): Promise<Lock> {
// The logic to acquire an exclusive lock, while avoiding race conditions is as follow:
//
// - Check if there is a lock file present
@ -209,13 +232,18 @@ export default class LockHandler {
//
// -If there is no lock file, create one with my identifier and try the whole cycle again to avoid race condition (re-check that the lock file is really mine)-.
options = {
...defaultAcquireLockOptions(),
...options,
};
const startTime = Date.now();
async function waitForTimeout() {
if (!timeoutMs) return false;
if (!options.timeoutMs) return false;
const elapsed = Date.now() - startTime;
if (timeoutMs && elapsed < timeoutMs) {
if (options.timeoutMs && elapsed < options.timeoutMs) {
await time.sleep(2);
return true;
}
@ -230,8 +258,12 @@ export default class LockHandler {
]);
if (activeSyncLock) {
if (await waitForTimeout()) continue;
throw new JoplinError(`Cannot acquire exclusive lock because the following clients have a sync lock on the target: ${this.lockToClientString(activeSyncLock)}`, 'hasSyncLock');
if (options.clearExistingSyncLocksFromTheSameClient && activeSyncLock.clientId === clientId && activeSyncLock.clientType === clientType) {
await this.releaseLock(LockType.Sync, clientType, clientId);
} else {
if (await waitForTimeout()) continue;
throw new JoplinError(`Cannot acquire exclusive lock because the following clients have a sync lock on the target: ${this.lockToClientString(activeSyncLock)}`, 'hasSyncLock');
}
}
if (activeExclusiveLock) {
@ -336,17 +368,22 @@ export default class LockHandler {
delete this.refreshTimers_[handle];
}
async acquireLock(lockType: LockType, clientType: string, clientId: string, timeoutMs: number = 0): Promise<Lock> {
public async acquireLock(lockType: LockType, clientType: string, clientId: string, options: AcquireLockOptions = null): Promise<Lock> {
options = {
...defaultAcquireLockOptions(),
...options,
};
if (lockType === LockType.Sync) {
return this.acquireSyncLock(clientType, clientId);
} else if (lockType === LockType.Exclusive) {
return this.acquireExclusiveLock(clientType, clientId, timeoutMs);
return this.acquireExclusiveLock(clientType, clientId, options);
} else {
throw new Error(`Invalid lock type: ${lockType}`);
}
}
async releaseLock(lockType: LockType, clientType: string, clientId: string) {
public async releaseLock(lockType: LockType, clientType: string, clientId: string) {
await this.api_.delete(this.lockFilePath({
type: lockType,
clientType: clientType,

6
packages/lib/services/synchronizer/MigrationHandler.ts
View File

@ -106,7 +106,11 @@ export default class MigrationHandler extends BaseService {
}
this.logger().info('MigrationHandler: Acquiring exclusive lock');
const exclusiveLock = await this.lockHandler_.acquireLock(LockType.Exclusive, this.clientType_, this.clientId_, 1000 * 30);
const exclusiveLock = await this.lockHandler_.acquireLock(LockType.Exclusive, this.clientType_, this.clientId_, {
clearExistingSyncLocksFromTheSameClient: true,
timeoutMs: 1000 * 30,
});
let autoLockError = null;
this.lockHandler_.startAutoLockRefresh(exclusiveLock, (error: any) => {
autoLockError = error;

17
packages/lib/services/synchronizer/synchronizer_LockHandler.test.ts
View File

@ -149,6 +149,23 @@ describe('synchronizer_LockHandler', function() {
}
}));
it('should ignore locks by same client when trying to acquire exclusive lock', (async () => {
const lockHandler = newLockHandler();
await lockHandler.acquireLock(LockType.Sync, 'desktop', '111');
await expectThrow(async () => {
await lockHandler.acquireLock(LockType.Exclusive, 'desktop', '111', { clearExistingSyncLocksFromTheSameClient: false });
}, 'hasSyncLock');
await expectNotThrow(async () => {
await lockHandler.acquireLock(LockType.Exclusive, 'desktop', '111', { clearExistingSyncLocksFromTheSameClient: true });
});
const activeLock = await lockHandler.activeLock(LockType.Exclusive);
expect(activeLock.clientId).toBe('111');
}));
// it('should not have race conditions', (async () => {
// const lockHandler = newLockHandler();