From 51235f191daf642b004b22f72bdcf2c352180f81 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Fri, 10 Jul 2020 23:42:03 +0100 Subject: [PATCH] All: Add support for sync target lock The goal is to allow locking a sync target so that maintenance operations, such as upgrading the target to a more efficient format, can be done. For now, only the lock mechanism is in place, as a way to evaluate it, and to see if it can cause any issue. --- .eslintignore | 2 + .gitignore | 2 + CliClient/gulpfile.js | 2 + CliClient/tests/synchronizer.js | 32 +-- CliClient/tests/synchronizer_LockHandler.ts | 104 +++++++++ CliClient/tests/test-utils.js | 46 +++- ElectronClient/package-lock.json | 6 + ElectronClient/package.json | 1 + .../lib/file-api-driver-memory.js | 2 +- .../lib/file-api-driver-webdav.js | 2 +- ReactNativeClient/lib/file-api.js | 5 + .../lib/services/synchronizer/LockHandler.ts | 220 ++++++++++++++++++ ReactNativeClient/lib/synchronizer.js | 157 +++++++------ package-lock.json | 22 ++ package.json | 2 + tsconfig.json | 4 + 16 files changed, 522 insertions(+), 87 deletions(-) create mode 100644 CliClient/tests/synchronizer_LockHandler.ts create mode 100644 ReactNativeClient/lib/services/synchronizer/LockHandler.ts diff --git a/.eslintignore b/.eslintignore index d79232c44..002420d13 100644 --- a/.eslintignore +++ b/.eslintignore @@ -61,6 +61,7 @@ Modules/TinyMCE/IconPack/postinstall.js Modules/TinyMCE/langs/ # AUTO-GENERATED - EXCLUDED TYPESCRIPT BUILD +CliClient/tests/synchronizer_LockHandler.js ElectronClient/commands/focusElement.js ElectronClient/commands/startExternalEditing.js ElectronClient/commands/stopExternalEditing.js @@ -154,6 +155,7 @@ ReactNativeClient/lib/services/ResourceEditWatcher.js ReactNativeClient/lib/services/rest/actionApi.desktop.js ReactNativeClient/lib/services/rest/errors.js ReactNativeClient/lib/services/SettingUtils.js +ReactNativeClient/lib/services/synchronizer/LockHandler.js ReactNativeClient/lib/services/UndoRedoService.js ReactNativeClient/lib/ShareExtension.js ReactNativeClient/lib/shareHandler.js diff --git a/.gitignore b/.gitignore index b08e40c5a..c3d4bcb02 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,7 @@ Tools/commit_hook.txt *.map # AUTO-GENERATED - EXCLUDED TYPESCRIPT BUILD +CliClient/tests/synchronizer_LockHandler.js ElectronClient/commands/focusElement.js ElectronClient/commands/startExternalEditing.js ElectronClient/commands/stopExternalEditing.js @@ -144,6 +145,7 @@ ReactNativeClient/lib/services/ResourceEditWatcher.js ReactNativeClient/lib/services/rest/actionApi.desktop.js ReactNativeClient/lib/services/rest/errors.js ReactNativeClient/lib/services/SettingUtils.js +ReactNativeClient/lib/services/synchronizer/LockHandler.js ReactNativeClient/lib/services/UndoRedoService.js ReactNativeClient/lib/ShareExtension.js ReactNativeClient/lib/shareHandler.js diff --git a/CliClient/gulpfile.js b/CliClient/gulpfile.js index 191e81dfe..2dc7c5d3f 100644 --- a/CliClient/gulpfile.js +++ b/CliClient/gulpfile.js @@ -37,6 +37,8 @@ tasks.buildTests = { 'lib/', 'locales/', 'node_modules/', + '*.ts', + '*.tsx', ], }); diff --git a/CliClient/tests/synchronizer.js b/CliClient/tests/synchronizer.js index edcfec69d..629da5436 100644 --- a/CliClient/tests/synchronizer.js +++ b/CliClient/tests/synchronizer.js @@ -33,7 +33,7 @@ async function allNotesFolders() { } async function remoteItemsByTypes(types) { - const list = await fileApi().list(); + const list = await fileApi().list('', { includeDirs: false }); if (list.has_more) throw new Error('Not implemented!!!'); const files = list.items; @@ -822,7 +822,7 @@ describe('synchronizer', function() { // First create a folder, without encryption enabled, and sync it const folder1 = await Folder.save({ title: 'folder1' }); await synchronizer().start(); - let files = await fileApi().list(); + let files = await fileApi().list('', { includeDirs: false }); let content = await fileApi().get(files.items[0].path); expect(content.indexOf('folder1') >= 0).toBe(true); @@ -1662,22 +1662,22 @@ describe('synchronizer', function() { expect(hasThrown).toBe(true); })); - it('should not sync when target is locked', asyncTest(async () => { - await synchronizer().start(); - await synchronizer().acquireLock_(); + // it('should not sync when target is locked', asyncTest(async () => { + // await synchronizer().start(); + // await synchronizer().acquireLock_(); - await switchClient(2); - const hasThrown = await checkThrowAsync(async () => synchronizer().start({ throwOnError: true })); - expect(hasThrown).toBe(true); - })); + // await switchClient(2); + // const hasThrown = await checkThrowAsync(async () => synchronizer().start({ throwOnError: true })); + // expect(hasThrown).toBe(true); + // })); - it('should clear a lock if it was created by the same app as the current one', asyncTest(async () => { - await synchronizer().start(); - await synchronizer().acquireLock_(); - expect((await synchronizer().lockFiles_()).length).toBe(1); - await synchronizer().start({ throwOnError: true }); - expect((await synchronizer().lockFiles_()).length).toBe(0); - })); + // it('should clear a lock if it was created by the same app as the current one', asyncTest(async () => { + // await synchronizer().start(); + // await synchronizer().acquireLock_(); + // expect((await synchronizer().lockFiles_()).length).toBe(1); + // await synchronizer().start({ throwOnError: true }); + // expect((await synchronizer().lockFiles_()).length).toBe(0); + // })); it('should not encrypt notes that are shared', asyncTest(async () => { Setting.setValue('encryption.enabled', true); diff --git a/CliClient/tests/synchronizer_LockHandler.ts b/CliClient/tests/synchronizer_LockHandler.ts new file mode 100644 index 000000000..1e2dbf974 --- /dev/null +++ b/CliClient/tests/synchronizer_LockHandler.ts @@ -0,0 +1,104 @@ +import LockHandler, { LockType } from 'lib/services/synchronizer/LockHandler'; + +require('app-module-path').addPath(__dirname); + +const { asyncTest, fileApi, setupDatabaseAndSynchronizer, switchClient, msleep, expectThrow, expectNotThrow } = require('test-utils.js'); + +process.on('unhandledRejection', (reason:any, p:any) => { + console.log('Unhandled Rejection at: Promise', p, 'reason:', reason); +}); + +let lockHandler_:LockHandler = null; +const locksDirname = 'locks'; + +function lockHandler():LockHandler { + if (lockHandler_) return lockHandler_; + lockHandler_ = new LockHandler(fileApi(), locksDirname); + return lockHandler_; +} + +describe('synchronizer_LockHandler', function() { + + beforeEach(async (done:Function) => { + lockHandler_ = null; + await setupDatabaseAndSynchronizer(1); + await setupDatabaseAndSynchronizer(2); + await switchClient(1); + done(); + }); + + it('should acquire and release a sync lock', asyncTest(async () => { + await lockHandler().acquireLock(LockType.Sync, 'mobile', '123456'); + const locks = await lockHandler().syncLocks(); + expect(locks.length).toBe(1); + expect(locks[0].type).toBe(LockType.Sync); + expect(locks[0].clientId).toBe('123456'); + expect(locks[0].clientType).toBe('mobile'); + + await lockHandler().releaseLock(LockType.Sync, 'mobile', '123456'); + expect((await lockHandler().syncLocks()).length).toBe(0); + })); + + it('should allow multiple sync locks', asyncTest(async () => { + await lockHandler().acquireLock(LockType.Sync, 'mobile', '111'); + + await switchClient(2); + + await lockHandler().acquireLock(LockType.Sync, 'mobile', '222'); + + expect((await lockHandler().syncLocks()).length).toBe(2); + + { + await lockHandler().releaseLock(LockType.Sync, 'mobile', '222'); + const locks = await lockHandler().syncLocks(); + expect(locks.length).toBe(1); + expect(locks[0].clientId).toBe('111'); + } + })); + + it('should refresh sync lock timestamp when acquiring again', asyncTest(async () => { + await lockHandler().acquireLock(LockType.Sync, 'mobile', '111'); + + const beforeTime = (await lockHandler().syncLocks())[0].updatedTime; + await msleep(1); + + await lockHandler().acquireLock(LockType.Sync, 'mobile', '111'); + + const afterTime = (await lockHandler().syncLocks())[0].updatedTime; + + expect(beforeTime).toBeLessThan(afterTime); + })); + + it('should not allow sync locks if there is an exclusive lock', asyncTest(async () => { + await lockHandler().acquireLock(LockType.Exclusive, 'desktop', '111'); + + expectThrow(async () => { + await lockHandler().acquireLock(LockType.Sync, 'mobile', '222'); + }, 'hasExclusiveLock'); + })); + + it('should not allow exclusive lock if there are sync locks', asyncTest(async () => { + lockHandler().syncLockMaxAge = 1000 * 60 * 60; + + await lockHandler().acquireLock(LockType.Sync, 'mobile', '111'); + await lockHandler().acquireLock(LockType.Sync, 'mobile', '222'); + + expectThrow(async () => { + await lockHandler().acquireLock(LockType.Exclusive, 'desktop', '333'); + }, 'hasSyncLock'); + })); + + it('should allow exclusive lock if the sync locks have expired', asyncTest(async () => { + lockHandler().syncLockMaxAge = 1; + + await lockHandler().acquireLock(LockType.Sync, 'mobile', '111'); + await lockHandler().acquireLock(LockType.Sync, 'mobile', '222'); + + await msleep(2); + + expectNotThrow(async () => { + await lockHandler().acquireLock(LockType.Exclusive, 'desktop', '333'); + }); + })); + +}); diff --git a/CliClient/tests/test-utils.js b/CliClient/tests/test-utils.js index c1450e1eb..14cb666d5 100644 --- a/CliClient/tests/test-utils.js +++ b/CliClient/tests/test-utils.js @@ -132,6 +132,14 @@ function sleep(n) { }); } +function msleep(ms) { + return new Promise((resolve, reject) => { + setTimeout(() => { + resolve(); + }, ms); + }); +} + function currentClientId() { return currentClient_; } @@ -378,6 +386,40 @@ async function checkThrowAsync(asyncFn) { return hasThrown; } +async function expectThrow(asyncFn, errorCode = undefined) { + let hasThrown = false; + let thrownErrorCode = null; + try { + await asyncFn(); + } catch (error) { + hasThrown = true; + thrownErrorCode = error.code; + } + + if (!hasThrown) { + expect('not throw').toBe('throw', 'Expected function to throw an error but did not'); + } else if (thrownErrorCode !== errorCode) { + expect(`error code: ${thrownErrorCode}`).toBe(`error code: ${errorCode}`); + } else { + expect(true).toBe(true); + } +} + +async function expectNotThrow(asyncFn) { + let thrownError = null; + try { + await asyncFn(); + } catch (error) { + thrownError = error; + } + + if (thrownError) { + expect(thrownError.message).toBe('', 'Expected function not to throw an error but it did'); + } else { + expect(true).toBe(true); + } +} + function checkThrow(fn) { let hasThrown = false; try { @@ -415,7 +457,7 @@ function asyncTest(callback) { } async function allSyncTargetItemsEncrypted() { - const list = await fileApi().list(); + const list = await fileApi().list('', { includeDirs: false }); const files = list.items; let totalCount = 0; @@ -573,4 +615,4 @@ class TestApp extends BaseApplication { } } -module.exports = { kvStore, resourceService, resourceFetcher, tempFilePath, allSyncTargetItemsEncrypted, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, checkThrow, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, asyncTest, currentClientId, id, ids, sortedIds, at, createNTestNotes, createNTestFolders, createNTestTags, TestApp }; +module.exports = { kvStore, expectThrow, expectNotThrow, resourceService, resourceFetcher, tempFilePath, allSyncTargetItemsEncrypted, msleep, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, checkThrow, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, asyncTest, currentClientId, id, ids, sortedIds, at, createNTestNotes, createNTestFolders, createNTestTags, TestApp }; diff --git a/ElectronClient/package-lock.json b/ElectronClient/package-lock.json index 0af931858..a73d641ba 100644 --- a/ElectronClient/package-lock.json +++ b/ElectronClient/package-lock.json @@ -235,6 +235,12 @@ "integrity": "sha512-Q1y515GcOdTHgagaVFhHnIFQ38ygs/kmxdNpvpou+raI9UO3YZcHDngBSYKQklcKlvA7iuQlmIKbzvmxcOE9CQ==", "dev": true }, + "@types/jasmine": { + "version": "3.5.11", + "resolved": "https://registry.npmjs.org/@types/jasmine/-/jasmine-3.5.11.tgz", + "integrity": "sha512-fg1rOd/DehQTIJTifGqGVY6q92lDgnLfs7C6t1ccSwQrMyoTGSoH6wWzhJDZb6ezhsdwAX4EIBLe8w5fXWmEng==", + "dev": true + }, "@types/node": { "version": "12.12.38", "resolved": "https://registry.npmjs.org/@types/node/-/node-12.12.38.tgz", diff --git a/ElectronClient/package.json b/ElectronClient/package.json index 6e365eada..d21db08f1 100644 --- a/ElectronClient/package.json +++ b/ElectronClient/package.json @@ -79,6 +79,7 @@ }, "homepage": "https://github.com/laurent22/joplin#readme", "devDependencies": { + "@types/jasmine": "^3.5.11", "ajv": "^6.5.0", "app-builder-bin": "^1.9.11", "babel-cli": "^6.26.0", diff --git a/ReactNativeClient/lib/file-api-driver-memory.js b/ReactNativeClient/lib/file-api-driver-memory.js index 762a81e0c..568ef3aab 100644 --- a/ReactNativeClient/lib/file-api-driver-memory.js +++ b/ReactNativeClient/lib/file-api-driver-memory.js @@ -111,7 +111,7 @@ class FileApiDriverMemory { this.items_.push(item); } else { this.items_[index].content = this.encodeContent_(content); - this.items_[index].updated_time = time.unix(); + this.items_[index].updated_time = time.unixMs(); } } diff --git a/ReactNativeClient/lib/file-api-driver-webdav.js b/ReactNativeClient/lib/file-api-driver-webdav.js index f43ec498d..ace44635a 100644 --- a/ReactNativeClient/lib/file-api-driver-webdav.js +++ b/ReactNativeClient/lib/file-api-driver-webdav.js @@ -89,7 +89,7 @@ class FileApiDriverWebDav { async delta(path, options) { const getDirStats = async path => { - const result = await this.list(path); + const result = await this.list(path, { includeDirs: false }); return result.items; }; diff --git a/ReactNativeClient/lib/file-api.js b/ReactNativeClient/lib/file-api.js index 754960090..6f545720f 100644 --- a/ReactNativeClient/lib/file-api.js +++ b/ReactNativeClient/lib/file-api.js @@ -128,6 +128,7 @@ class FileApi { if (!options) options = {}; if (!('includeHidden' in options)) options.includeHidden = false; if (!('context' in options)) options.context = null; + if (!('includeDirs' in options)) options.includeDirs = true; this.logger().debug(`list ${this.baseDir()}`); @@ -141,6 +142,10 @@ class FileApi { result.items = temp; } + if (!options.includeHidden) { + result.items = result.items.filter(f => !f.isDir); + } + return result; } diff --git a/ReactNativeClient/lib/services/synchronizer/LockHandler.ts b/ReactNativeClient/lib/services/synchronizer/LockHandler.ts new file mode 100644 index 000000000..bbacaa773 --- /dev/null +++ b/ReactNativeClient/lib/services/synchronizer/LockHandler.ts @@ -0,0 +1,220 @@ +const JoplinError = require('lib/JoplinError'); +const { time } = require('lib/time-utils'); +const { fileExtension, filename } = require('lib/path-utils.js'); + +export enum LockType { + None = '', + Sync = 'sync', + Exclusive = 'exclusive', +} + +export interface Lock { + type: LockType, + clientType: string, + clientId: string, + updatedTime?: number, +} + +const exclusiveFilename = 'exclusive.json'; + +export default class LockHandler { + + private api_:any = null; + private lockDirPath_:any = null; + private syncLockMaxAge_:number = 1000 * 60 * 3; + + constructor(api:any, lockDirPath:string) { + this.api_ = api; + this.lockDirPath_ = lockDirPath; + } + + public get syncLockMaxAge():number { + return this.syncLockMaxAge_; + } + + // Should only be done for testing purposes since all clients should + // use the same lock max age. + public set syncLockMaxAge(v:number) { + this.syncLockMaxAge_ = v; + } + + private lockFilename(lock:Lock) { + if (lock.type === LockType.Exclusive) { + return exclusiveFilename; + } else { + return `${[lock.type, lock.clientType, lock.clientId].join('_')}.json`; + } + } + + private lockTypeFromFilename(name:string):LockType { + if (name === exclusiveFilename) return LockType.Exclusive; + return LockType.Sync; + } + + private lockFilePath(lock:Lock) { + return `${this.lockDirPath_}/${this.lockFilename(lock)}`; + } + + private exclusiveFilePath():string { + return `${this.lockDirPath_}/${exclusiveFilename}`; + } + + private syncLockFileToObject(file:any):Lock { + const p = filename(file.path).split('_'); + + return { + type: p[0], + clientType: p[1], + clientId: p[2], + updatedTime: file.updated_time, + }; + } + + async syncLocks():Promise { + const result = await this.api_.list(this.lockDirPath_); + if (result.hasMore) throw new Error('hasMore not handled'); // Shouldn't happen anyway + + const output = []; + for (const file of result.items) { + const ext = fileExtension(file.path); + if (ext !== 'json') continue; + + const type = this.lockTypeFromFilename(file.path); + if (type !== LockType.Sync) continue; + + const lock = this.syncLockFileToObject(file); + output.push(lock); + } + + return output; + } + + private async exclusiveLock():Promise { + const stat = await this.api_.stat(this.exclusiveFilePath()); + if (!stat) return null; + + const contentText = await this.api_.get(this.exclusiveFilePath()); + if (!contentText) return null; // race condition + + const lock:Lock = JSON.parse(contentText) as Lock; + lock.updatedTime = stat.updated_time; + return lock; + } + + private lockIsActive(lock:Lock):boolean { + return Date.now() - lock.updatedTime < this.syncLockMaxAge; + } + + async hasActiveExclusiveLock():Promise { + const lock = await this.exclusiveLock(); + return !!lock && this.lockIsActive(lock); + } + + async hasActiveSyncLock(clientType:string, clientId:string) { + const locks = await this.syncLocks(); + for (const lock of locks) { + if (lock.clientType === clientType && lock.clientId === clientId && this.lockIsActive(lock)) return true; + } + return false; + } + + private async saveLock(lock:Lock) { + await this.api_.put(this.lockFilePath(lock), JSON.stringify(lock)); + } + + private async acquireSyncLock(clientType:string, clientId:string) { + const exclusiveLock = await this.exclusiveLock(); + + if (exclusiveLock) { + throw new JoplinError(`Cannot acquire sync lock because the following client has an exclusive lock on the sync target: ${this.lockToClientString(exclusiveLock)}`, 'hasExclusiveLock'); + } + + await this.saveLock({ + type: LockType.Sync, + clientType: clientType, + clientId: clientId, + }); + } + + private lockToClientString(lock:Lock):string { + return `(${lock.clientType} #${lock.clientId})`; + } + + private async acquireExclusiveLock(clientType:string, clientId:string, timeoutMs:number = 0) { + // The logic to acquire an exclusive lock, while avoiding race conditions is as follow: + // + // - Check if there is a lock file present + // + // - If there is a lock file, see if I'm the one owning it by checking that its content has my identifier. + // - If that's the case, just write to the data file then delete the lock file. + // - If that's not the case, just wait a second or a small random length of time and try the whole cycle again-. + // + // -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)-. + + const startTime = Date.now(); + + async function waitForTimeout() { + if (!timeoutMs) return false; + + const elapsed = Date.now() - startTime; + if (timeoutMs && elapsed < timeoutMs) { + await time.sleep(2); + return true; + } + return false; + } + + while (true) { + const syncLocks = await this.syncLocks(); + const activeSyncLocks = syncLocks.filter(lock => this.lockIsActive(lock)); + + if (activeSyncLocks.length) { + if (await waitForTimeout()) continue; + const lockString = activeSyncLocks.map(l => this.lockToClientString(l)).join(', '); + throw new JoplinError(`Cannot acquire exclusive lock because the following clients have a sync lock on the target: ${lockString}`, 'hasSyncLock'); + } + + const exclusiveLock = await this.exclusiveLock(); + + if (exclusiveLock) { + if (exclusiveLock.clientId === clientId) { + // Save it again to refresh the timestamp + await this.saveLock(exclusiveLock); + return; + } else { + // If there's already an exclusive lock, wait for it to be released + if (await waitForTimeout()) continue; + throw new JoplinError(`Cannot acquire exclusive lock because the following client has an exclusive lock on the sync target: ${this.lockToClientString(exclusiveLock)}`, 'hasExclusiveLock'); + } + } else { + // If there's not already an exclusive lock, acquire one + // then loop again to check that we really got the lock + // (to prevent race conditions) + await this.saveLock({ + type: LockType.Exclusive, + clientType: clientType, + clientId: clientId, + }); + } + } + } + + async acquireLock(lockType:LockType, clientType:string, clientId:string, timeoutMs:number = 0) { + if (lockType === LockType.Sync) { + await this.acquireSyncLock(clientType, clientId); + } else if (lockType === LockType.Exclusive) { + await this.acquireExclusiveLock(clientType, clientId, timeoutMs); + } else { + throw new Error(`Invalid lock type: ${lockType}`); + } + } + + async releaseLock(lockType:LockType, clientType:string, clientId:string) { + await this.api_.delete(this.lockFilePath({ + type: lockType, + clientType: clientType, + clientId: clientId, + })); + } + +} diff --git a/ReactNativeClient/lib/synchronizer.js b/ReactNativeClient/lib/synchronizer.js index 56e7f08ac..01a2f5867 100644 --- a/ReactNativeClient/lib/synchronizer.js +++ b/ReactNativeClient/lib/synchronizer.js @@ -12,10 +12,11 @@ const { time } = require('lib/time-utils.js'); const { Logger } = require('lib/logger.js'); const { _ } = require('lib/locale.js'); const { shim } = require('lib/shim.js'); -const { filename, fileExtension } = require('lib/path-utils'); +// const { filename, fileExtension } = require('lib/path-utils'); const JoplinError = require('lib/JoplinError'); const BaseSyncTarget = require('lib/BaseSyncTarget'); const TaskQueue = require('lib/TaskQueue'); +const LockHandler = require('lib/services/synchronizer/LockHandler').default; class Synchronizer { constructor(db, api, appType) { @@ -23,7 +24,7 @@ class Synchronizer { this.db_ = db; this.api_ = api; this.syncDirName_ = '.sync'; - this.lockDirName_ = '.lock'; + this.lockDirName_ = 'locks'; this.resourceDirName_ = BaseSyncTarget.resourceDirName(); this.logger_ = new Logger(); this.appType_ = appType; @@ -66,6 +67,12 @@ class Synchronizer { return this.logger_; } + lockHandler() { + if (this.lockHandler_) return this.lockHandler_; + this.lockHandler_ = new LockHandler(this.api(), this.lockDirName_); + return this.lockHandler_; + } + maxResourceSize() { if (this.maxResourceSize_ !== null) return this.maxResourceSize_; return this.appType_ === 'mobile' ? 10 * 1000 * 1000 : Infinity; @@ -205,64 +212,17 @@ class Synchronizer { return state; } - async acquireLock_() { - await this.checkLock_(); - await this.api().put(`${this.lockDirName_}/${this.clientId()}_${Date.now()}.lock`, `${Date.now()}`); - } - - async releaseLock_() { - const lockFiles = await this.lockFiles_(); - for (const lockFile of lockFiles) { - const p = this.parseLockFilePath(lockFile.path); - if (p.clientId === this.clientId()) { - await this.api().delete(p.fullPath); - } - } - } - - async lockFiles_() { - const output = await this.api().list(this.lockDirName_); - return output.items.filter((p) => { - const ext = fileExtension(p.path); - return ext === 'lock'; - }); - } - - parseLockFilePath(path) { - const splitted = filename(path).split('_'); - const fullPath = `${this.lockDirName_}/${path}`; - if (splitted.length !== 2) throw new Error(`Sync target appears to be locked but lock filename is invalid: ${fullPath}. Please delete it on the sync target to continue.`); - return { - clientId: splitted[0], - timestamp: Number(splitted[1]), - fullPath: fullPath, - }; - } - - async checkLock_() { - const lockFiles = await this.lockFiles_(); - if (lockFiles.length) { - const lock = this.parseLockFilePath(lockFiles[0].path); - - if (lock.clientId === this.clientId()) { - await this.releaseLock_(); - } else { - throw new Error(`The sync target was locked by client ${lock.clientId} on ${time.unixMsToLocalDateTime(lock.timestamp)} and cannot be accessed. If no app is currently operating on the sync target, you can delete the files in the "${this.lockDirName_}" directory on the sync target to resume.`); - } - } - } - async checkSyncTargetVersion_() { const supportedSyncTargetVersion = Setting.value('syncVersion'); - const syncTargetVersion = await this.api().get('.sync/version.txt'); + const syncTargetVersion = await this.apiCall('get', '.sync/version.txt'); if (!syncTargetVersion) { - await this.api().put('.sync/version.txt', `${supportedSyncTargetVersion}`); + await this.apiCall('put', '.sync/version.txt', `${supportedSyncTargetVersion}`); } else { if (Number(syncTargetVersion) > supportedSyncTargetVersion) { throw new Error(sprintf('Sync version of the target (%d) does not match sync version supported by client (%d). Please upgrade your client.', Number(syncTargetVersion), supportedSyncTargetVersion)); } else { - await this.api().put('.sync/version.txt', `${supportedSyncTargetVersion}`); + await this.apiCall('put', '.sync/version.txt', `${supportedSyncTargetVersion}`); // TODO: do upgrade job } } @@ -272,6 +232,44 @@ class Synchronizer { return steps.includes('update_remote') && steps.includes('delete_remote') && steps.includes('delta'); } + async lockErrorStatus_() { + const hasActiveExclusiveLock = await this.lockHandler().hasActiveExclusiveLock(); + if (hasActiveExclusiveLock) return 'hasExclusiveLock'; + + const hasActiveSyncLock = await this.lockHandler().hasActiveSyncLock(); + if (!hasActiveSyncLock) return 'syncLockGone'; + + return ''; + } + + async apiCall(fnName, ...args) { + if (this.syncTargetIsLocked_) throw new JoplinError('Sync target is locked - aborting API call', 'lockError'); + + try { + const output = await this.api()[fnName](...args); + return output; + } catch (error) { + const lockStatus = await this.lockErrorStatus_(); + // When there's an error due to a lock, we re-wrap the error and change the error code so that error handling + // does not do special processing on the original error. For example, if a resource could not be downloaded, + // don't mark it as a "cannotSyncItem" since we don't know that. + if (lockStatus) { + throw new JoplinError(`Sync target lock error: ${lockStatus}. Original error was: ${error.message}`, 'lockError'); + } else { + throw error; + } + } + } + + async refreshLock() { + if (this.state_ !== 'in_progress') { + this.logger().warn('Trying to refresh lock, but sync is not in progress'); + return; + } + + await this.lockHandler().acquireLock('sync', this.appType_, this.clientId_); + } + // Synchronisation is done in three major steps: // // 1. UPLOAD: Send to the sync target the items that have changed since the last sync. @@ -300,6 +298,7 @@ class Synchronizer { const syncTargetId = this.api().syncTargetId(); + this.syncTargetIsLocked_ = false; this.cancelling_ = false; const masterKeysBefore = await MasterKey.count(); @@ -325,12 +324,27 @@ class Synchronizer { let errorToThrow = null; try { - await this.api().mkdir(this.syncDirName_); - await this.api().mkdir(this.lockDirName_); + await this.apiCall('mkdir', this.syncDirName_); + await this.apiCall('mkdir', this.lockDirName_); this.api().setTempDirName(this.syncDirName_); - await this.api().mkdir(this.resourceDirName_); + await this.apiCall('mkdir', this.resourceDirName_); + + await this.lockHandler().acquireLock('sync', this.appType_, this.clientId_); + + if (this.refreshLockIID_) clearInterval(this.refreshLockIID_); + + this.refreshLockIID_ = setInterval(async () => { + try { + await this.refreshLock(); + } catch (error) { + this.logger().warn('Could not refresh lock - cancelling sync. Error was:', error); + clearInterval(this.refreshLockIID_); + this.syncTargetIsLocked_ = true; + this.refreshLockIID_ = null; + this.cancel(); + } + }, 1000 * 60); - await this.checkLock_(); await this.checkSyncTargetVersion_(); // ======================================================================== @@ -369,7 +383,7 @@ class Synchronizer { // (by setting an updated_time less than current time). if (donePaths.indexOf(path) >= 0) throw new JoplinError(sprintf('Processing a path that has already been done: %s. sync_time was not updated? Remote item has an updated_time in the future?', path), 'processingPathTwice'); - const remote = await this.api().stat(path); + const remote = await this.apiCall('stat', path); let action = null; let reason = ''; @@ -407,7 +421,7 @@ class Synchronizer { // OneDrive does not appear to have accurate timestamps as lastModifiedDateTime would occasionally be // a few seconds ahead of what it was set with setTimestamp() try { - remoteContent = await this.api().get(path); + remoteContent = await this.apiCall('get', path); } catch (error) { if (error.code === 'rejectedByTarget') { this.progressReport_.errors.push(error); @@ -450,7 +464,7 @@ class Synchronizer { this.logger().warn(`Uploading a large resource (resourceId: ${local.id}, size:${local.size} bytes) which may tie up the sync process.`); } - await this.api().put(remoteContentPath, null, { path: localResourceContentPath, source: 'file' }); + await this.apiCall('put', remoteContentPath, null, { path: localResourceContentPath, source: 'file' }); } catch (error) { if (error && ['rejectedByTarget', 'fileNotFound'].indexOf(error.code) >= 0) { await handleCannotSyncItem(ItemClass, syncTargetId, local, error.message); @@ -467,7 +481,7 @@ class Synchronizer { try { if (this.testingHooks_.indexOf('notesRejectedByTarget') >= 0 && local.type_ === BaseModel.TYPE_NOTE) throw new JoplinError('Testing rejectedByTarget', 'rejectedByTarget'); const content = await ItemClass.serializeForSync(local); - await this.api().put(path, content); + await this.apiCall('put', path, content); } catch (error) { if (error && error.code === 'rejectedByTarget') { await handleCannotSyncItem(ItemClass, syncTargetId, local, error.message); @@ -593,11 +607,11 @@ class Synchronizer { const item = deletedItems[i]; const path = BaseItem.systemPath(item.item_id); this.logSyncOperation('deleteRemote', null, { id: item.item_id }, 'local has been deleted'); - await this.api().delete(path); + await this.apiCall('delete', path); if (item.item_type === BaseModel.TYPE_RESOURCE) { const remoteContentPath = resourceRemotePath(item.item_id); - await this.api().delete(remoteContentPath); + await this.apiCall('delete', remoteContentPath); } await BaseItem.remoteDeletedItem(syncTargetId, item.item_id); @@ -628,7 +642,7 @@ class Synchronizer { while (true) { if (this.cancelling() || hasCancelled) break; - const listResult = await this.api().delta('', { + const listResult = await this.apiCall('delta', '', { context: context, // allItemIdsHandler() provides a way for drivers that don't have a delta API to @@ -653,7 +667,7 @@ class Synchronizer { if (this.cancelling()) break; this.downloadQueue_.push(remote.path, async () => { - return this.api().get(remote.path); + return this.apiCall('get', remote.path); }); } @@ -669,7 +683,7 @@ class Synchronizer { if (!BaseItem.isSystemPath(remote.path)) continue; // The delta API might return things like the .sync, .resource or the root folder const loadContent = async () => { - const task = await this.downloadQueue_.waitForResult(path); // await this.api().get(path); + const task = await this.downloadQueue_.waitForResult(path); // await this.apiCall('get', path); if (task.error) throw task.error; if (!task.result) return null; return await BaseItem.unserialize(task.result); @@ -832,13 +846,13 @@ class Synchronizer { } catch (error) { if (throwOnError) { errorToThrow = error; - } else if (error && ['cannotEncryptEncrypted', 'noActiveMasterKey', 'processingPathTwice', 'failSafe'].indexOf(error.code) >= 0) { + } else if (error && ['cannotEncryptEncrypted', 'noActiveMasterKey', 'processingPathTwice', 'failSafe', 'lockError'].indexOf(error.code) >= 0) { // Only log an info statement for this since this is a common condition that is reported // in the application, and needs to be resolved by the user. // Or it's a temporary issue that will be resolved on next sync. this.logger().info(error.message); - if (error.code === 'failSafe') { + if (error.code === 'failSafe' || error.code === 'lockError') { // Get the message to display on UI, but not in testing to avoid poluting stdout if (!shim.isTestingEnv()) this.progressReport_.errors.push(error.message); this.logLastRequests(); @@ -857,6 +871,15 @@ class Synchronizer { } } + await this.lockHandler().releaseLock('sync', this.appType_, this.clientId_); + + if (this.refreshLockIID_) { + clearInterval(this.refreshLockIID_); + this.refreshLockIID_ = null; + } + + this.syncTargetIsLocked_ = false; + if (this.cancelling()) { this.logger().info('Synchronisation was cancelled.'); this.cancelling_ = false; diff --git a/package-lock.json b/package-lock.json index 0a20d7b8c..704cebd70 100644 --- a/package-lock.json +++ b/package-lock.json @@ -92,6 +92,12 @@ "hoist-non-react-statics": "^3.3.0" } }, + "@types/jasmine": { + "version": "3.5.11", + "resolved": "https://registry.npmjs.org/@types/jasmine/-/jasmine-3.5.11.tgz", + "integrity": "sha512-fg1rOd/DehQTIJTifGqGVY6q92lDgnLfs7C6t1ccSwQrMyoTGSoH6wWzhJDZb6ezhsdwAX4EIBLe8w5fXWmEng==", + "dev": true + }, "@types/json-schema": { "version": "7.0.3", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.3.tgz", @@ -4189,6 +4195,22 @@ "resolved": "https://registry.npmjs.org/isstream/-/isstream-0.1.2.tgz", "integrity": "sha1-R+Y/evVa+m+S4VAOaQ64uFKcCZo=" }, + "jasmine": { + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/jasmine/-/jasmine-3.5.0.tgz", + "integrity": "sha512-DYypSryORqzsGoMazemIHUfMkXM7I7easFaxAvNM3Mr6Xz3Fy36TupTrAOxZWN8MVKEU5xECv22J4tUQf3uBzQ==", + "dev": true, + "requires": { + "glob": "^7.1.4", + "jasmine-core": "~3.5.0" + } + }, + "jasmine-core": { + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/jasmine-core/-/jasmine-core-3.5.0.tgz", + "integrity": "sha512-nCeAiw37MIMA9w9IXso7bRaLl+c/ef3wnxsoSAlYrzS+Ot0zTG6nU8G/cIfGkqpkjX2wNaIW9RFG0TwIFnG6bA==", + "dev": true + }, "joplin-turndown": { "version": "4.0.28", "resolved": "https://registry.npmjs.org/joplin-turndown/-/joplin-turndown-4.0.28.tgz", diff --git a/package.json b/package.json index e5d89cd0f..9f648a49e 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,7 @@ }, "license": "MIT", "devDependencies": { + "@types/jasmine": "^3.5.11", "@types/react": "^16.9.0", "@types/react-dom": "^16.9.0", "@types/react-redux": "^7.1.7", @@ -33,6 +34,7 @@ "glob": "^7.1.6", "gulp": "^4.0.2", "husky": "^3.0.2", + "jasmine": "^3.5.0", "lint-staged": "^9.2.1", "typescript": "^3.7.3" }, diff --git a/tsconfig.json b/tsconfig.json index fb3bf0186..e63bd4340 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -15,6 +15,10 @@ "sourceMap": true, "jsx": "react", "skipLibCheck": true, + "baseUrl": ".", + "paths": { + "lib/*": ["./ReactNativeClient/lib/*"], + }, }, "include": [ "ReactNativeClient/**/*",