All: Use mutex when saving model to avoid race conditions when decrypting and syncing at the same time

CliClient/package-lock.json

@@ -64,6 +64,11 @@
         }
       }
     },
+    "async-mutex": {
+      "version": "0.1.3",
+      "resolved": "https://registry.npmjs.org/async-mutex/-/async-mutex-0.1.3.tgz",
+      "integrity": "sha1-Cq0hEjaXlas/F+M3RFVtLs9UdWY="
+    },
     "asynckit": {
       "version": "0.4.0",
       "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz",

CliClient/package.json

@@ -28,6 +28,7 @@
   },
   "dependencies": {
     "app-module-path": "^2.2.0",
+    "async-mutex": "^0.1.3",
     "base-64": "^0.1.0",
     "compare-version": "^0.1.2",
     "follow-redirects": "^1.2.4",

ElectronClient/app/package-lock.json

@@ -237,6 +237,11 @@
       "integrity": "sha512-NW2cX8m1Q7KPA7a5M2ULQeZ2wR5qI5PAbw5L0UOMxdioVk9PMZ0h1TmyZEkPYrCvYjDlFICusOu1dlEKAAeXBw==",
       "dev": true
     },
+    "async-mutex": {
+      "version": "0.1.3",
+      "resolved": "https://registry.npmjs.org/async-mutex/-/async-mutex-0.1.3.tgz",
+      "integrity": "sha1-Cq0hEjaXlas/F+M3RFVtLs9UdWY="
+    },
     "asynckit": {
       "version": "0.4.0",
       "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz",

ElectronClient/app/package.json

@@ -55,6 +55,7 @@
   },
   "dependencies": {
     "app-module-path": "^2.2.0",
+    "async-mutex": "^0.1.3",
     "base-64": "^0.1.0",
     "electron-context-menu": "^0.9.1",
     "electron-log": "^2.2.11",

ReactNativeClient/lib/BaseModel.js

@@ -2,6 +2,7 @@ const { Log } = require('lib/log.js');
 const { Database } = require('lib/database.js');
 const { uuid } = require('lib/uuid.js');
 const { time } = require('lib/time-utils.js');
+const Mutex = require('async-mutex').Mutex;
 
 class BaseModel {
 
@@ -247,6 +248,40 @@ class BaseModel {
 		return !Object.getOwnPropertyNames(diff).length;
 	}
 
+	static saveMutex(modelOrId) {
+		const noLockMutex = {
+			acquire: function() { return null; }
+		};
+
+		if (!modelOrId) return noLockMutex;
+
+		let modelId = typeof modelOrId === 'string' ? modelOrId : modelOrId.id;
+
+		if (!modelId) return noLockMutex;
+
+		let mutex = BaseModel.saveMutexes_[modelId];
+		if (mutex) return mutex;
+
+		mutex = new Mutex();
+		BaseModel.saveMutexes_[modelId] = mutex;
+		return mutex;
+	}
+
+	static releaseSaveMutex(modelOrId, release) {
+		if (!release) return;
+		if (!modelOrId) return release();
+
+		let modelId = typeof modelOrId === 'string' ? modelOrId : modelOrId.id;
+
+		if (!modelId) return release();
+
+		let mutex = BaseModel.saveMutexes_[modelId];
+		if (!mutex) return release();
+
+		delete BaseModel.saveMutexes_[modelId];
+		release();
+	}
+
 	static saveQuery(o, options) {
 		let temp = {}
 		let fieldNames = this.fieldNames();
@@ -320,7 +355,16 @@ class BaseModel {
 		return query;
 	}
 
-	static save(o, options = null) {
+	static async save(o, options = null) {
+		// When saving, there's a mutex per model ID. This is because the model returned from this function
+		// is basically its input `o` (instead of being read from the database, for performance reasons).
+		// This works well in general except if that model is saved simultaneously in two places. In that
+		// case, the output won't be up-to-date and would cause for example display issues with out-dated
+		// notes being displayed. This was an issue when notes were being synchronised while being decrypted
+		// at the same time.
+
+		const mutexRelease = await this.saveMutex(o).acquire();
+
 		options = this.modOptions(options);
 		options.isNew = this.isNew(o, options);
 
@@ -348,7 +392,11 @@ class BaseModel {
 			queries = queries.concat(options.nextQueries);
 		}
 
-		return this.db().transactionExecBatch(queries).then(() => {
+		let output = null;
+
+		try {
+			await this.db().transactionExecBatch(queries);
+
 			o = Object.assign({}, o);
 			if (modelId) o.id = modelId;
 			if ('updated_time' in saveQuery.modObject) o.updated_time = saveQuery.modObject.updated_time;
@@ -365,10 +413,14 @@ class BaseModel {
 				}
 			}
 
-			return this.filter(o);
-		}).catch((error) => {
+			output = this.filter(o);
+		} catch (error) {
 			Log.error('Cannot save model', error);
-		});
+		}
+
+		this.releaseSaveMutex(o, mutexRelease);
+
+		return output;
 	}
 
 	static isNew(object, options) {
@@ -447,5 +499,6 @@ BaseModel.TYPE_MASTER_KEY = 9;
 
 BaseModel.db_ = null;
 BaseModel.dispatch = function(o) {};
+BaseModel.saveMutexes_ = {};
 
 module.exports = BaseModel;

ReactNativeClient/lib/models/Note.js

@@ -381,12 +381,13 @@ class Note extends BaseItem {
 		return this.save(newNote);
 	}
 
-	static save(o, options = null) {
+	static async save(o, options = null) {
 		let isNew = this.isNew(o, options);
 		if (isNew && !o.source) o.source = Setting.value('appName');
 		if (isNew && !o.source_application) o.source_application = Setting.value('appId');
 
-		return super.save(o, options).then((note) => {
+		const note = await super.save(o, options);
+
 		this.dispatch({
 			type: 'NOTE_UPDATE_ONE',
 			note: note,
@@ -400,7 +401,6 @@ class Note extends BaseItem {
 		}
 		
 		return note;
-		});
 	}
 
 	static async delete(id, options = null) {

ReactNativeClient/package-lock.json

@@ -228,6 +228,11 @@
         "lodash": "4.17.4"
       }
     },
+    "async-mutex": {
+      "version": "0.1.3",
+      "resolved": "https://registry.npmjs.org/async-mutex/-/async-mutex-0.1.3.tgz",
+      "integrity": "sha1-Cq0hEjaXlas/F+M3RFVtLs9UdWY="
+    },
     "asynckit": {
       "version": "0.4.0",
       "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz",

ReactNativeClient/package.json

@@ -10,6 +10,7 @@
     "postinstall": "node ../Tools/copycss.js"
   },
   "dependencies": {
+    "async-mutex": "^0.1.3",
     "base-64": "^0.1.0",
     "buffer": "^5.0.8",
     "events": "^1.1.1",