From 78ce10ddf08ed400f4f4537c41d602a34f074701 Mon Sep 17 00:00:00 2001
From: Laurent Cozic <laurent@cozic.net>
Date: Mon, 8 Jan 2018 20:09:01 +0100
Subject: [PATCH] All: Fixed race condition when a note is being uploaded while
 it's being modified in the text editor

---
 CliClient/tests/synchronizer.js                  |  2 ++
 ElectronClient/app/gui/NoteText.jsx              |  2 +-
 .../lib/services/DecryptionWorker.js             |  1 +
 ReactNativeClient/lib/synchronizer.js            | 16 +++++++++++++++-
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/CliClient/tests/synchronizer.js b/CliClient/tests/synchronizer.js
index 99fee8a3e2..af7fd03d2e 100644
--- a/CliClient/tests/synchronizer.js
+++ b/CliClient/tests/synchronizer.js
@@ -830,6 +830,8 @@ describe('Synchronizer', function() {
 	}));
 
 	it('should sync resources', asyncTest(async () => {
+		while (insideBeforeEach) await time.msleep(100);
+
 		let folder1 = await Folder.save({ title: "folder1" });
 		let note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });
 		await shim.attachFileToNote(note1, __dirname + '/../tests/support/photo.jpg');
diff --git a/ElectronClient/app/gui/NoteText.jsx b/ElectronClient/app/gui/NoteText.jsx
index 6bdf90176f..310c2fe8cb 100644
--- a/ElectronClient/app/gui/NoteText.jsx
+++ b/ElectronClient/app/gui/NoteText.jsx
@@ -167,7 +167,7 @@ class NoteTextComponent extends React.Component {
 	async componentWillReceiveProps(nextProps) {
 		if ('noteId' in nextProps && nextProps.noteId !== this.props.noteId) {
 			await this.reloadNote(nextProps);
-			if(this.editor_){
+			if (this.editor_){
 				const session = this.editor_.editor.getSession();
 				const undoManager = session.getUndoManager();
 				undoManager.reset();
diff --git a/ReactNativeClient/lib/services/DecryptionWorker.js b/ReactNativeClient/lib/services/DecryptionWorker.js
index a275d90e90..b1598db25c 100644
--- a/ReactNativeClient/lib/services/DecryptionWorker.js
+++ b/ReactNativeClient/lib/services/DecryptionWorker.js
@@ -83,6 +83,7 @@ class DecryptionWorker {
 							});
 							continue;
 						}
+						this.logger().warn('DecryptionWorker: error for: ' + item.id + ' (' + ItemClass.tableName() + ')');
 						throw error;
 					}
 				}
diff --git a/ReactNativeClient/lib/synchronizer.js b/ReactNativeClient/lib/synchronizer.js
index 2be9012e66..a976942f9b 100644
--- a/ReactNativeClient/lib/synchronizer.js
+++ b/ReactNativeClient/lib/synchronizer.js
@@ -320,9 +320,23 @@ class Synchronizer {
 							}
 						}
 
+						// Note: Currently, we set sync_time to update_time, which should work fine given that the resolution is the millisecond.
+						// In theory though, this could happen:
+						//
+						// 1. t0: Editor: Note is modified
+						// 2. t0: Sync: Found that note was modified so start uploading it
+						// 3. t0: Editor: Note is modified again
+						// 4. t1: Sync: Note has finished uploading, set sync_time to t0
+						//
+						// Later any attempt to sync will not detect that note was modified in (3) (within the same millisecond as it was being uploaded)
+						// because sync_time will be t0 too.
+						//
+						// The solution would be to use something like an etag (a simple counter incremented on every change) to make sure each
+						// change is uniquely identified. Leaving it like this for now.
+
 						if (canSync) {
 							await this.api().setTimestamp(path, local.updated_time);
-							await ItemClass.saveSyncTime(syncTargetId, local, time.unixMs());
+							await ItemClass.saveSyncTime(syncTargetId, local, local.updated_time);
 						}
 
 					} else if (action == 'itemConflict') {