Skip to content

Issue #159: Send final modifications before remove operation#170

Merged
roundhill merged 10 commits intoSimperium:developfrom
aforcier:issue/159-concurrency-exception
Apr 20, 2015
Merged

Issue #159: Send final modifications before remove operation#170
roundhill merged 10 commits intoSimperium:developfrom
aforcier:issue/159-concurrency-exception

Conversation

@aforcier
Copy link
Copy Markdown
Contributor

Fixes #159. The fix stores a backup copy of the object whenever BucketObject.save() is called. When ChangeProcessor requests the object to produce the diff, it's given the backup copy if the master is missing from the persistent store.

I found that there's a second way a remove operation can be sent to the server before a modification that preceded it. This is a concurrency issue caused by a multi-threaded Executor in Bucket sometimes running the remove() thread before the sync() thread, and can happen if there is nearly no delay between BucketObject.save() and BucketObject.delete() (for example, a 20ms delay between them was enough for the bug not to happen in testing on Simplenote). In this case the remove operation is sent to the ChangeProcessor first and the object is deleted on the server without its final modification.

I addressed the second issue by adding thread locking to sync() and remove() in Bucket, so that remove() always runs after sync() is completed for the same object. I also added a ConcurrencyTest unit test class, which tests this fix on a multi-threaded Executor.

- Added a method that will return the backup copy if retrieval from persistent store fails
- Fixes an issue found in Simplenote where deleting and trashing a note right after modifying it could cause the deletion not to reach the server. This was due to an object in the backup store having an outdated ghost.
…cket

- Promoted Channel.isIdle() to a method of the Bucket.Channel interface
- Added a unit test for consecutive saving and deleting of two notes (also tests backup store clearing)
- Fixes an issue where calling BucketObject.save() and then immediately BucketObject.remove() on a multi-core device could result in the remove operation being sent to the ChangeProcessor before the modify operation
…ception' into issue/159-concurrency-exception
@aforcier aforcier changed the title Issue/159 concurrency exception Issue #159: Send final modifications before remove operation Feb 16, 2015
@aforcier
Copy link
Copy Markdown
Contributor Author

Fixed a unit test that failed on Travis. The problem was that Travis' single-threaded emulator didn't process the threads fast enough for the fixed time the test was waiting. I changed the test to wait for ThreadPoolExecutor to finish instead.

@roundhill roundhill self-assigned this Mar 27, 2015
@roundhill
Copy link
Copy Markdown
Contributor

Looks ok, I hooked it up to Simplenote and syncing was functioning as it should although I couldn't get it to get into a situation where the lock was applied. @aforcier maybe you have some tips on how to get it to reproduce in Simplenote?

Otherwise the tests pass and the code looks good 👍

@beaucollins you can have final say before merging?

@aforcier
Copy link
Copy Markdown
Contributor Author

@roundhill, I wasn't able to reproduce the issue by normal usage of the app.

What I had to do in testing was modify the 'Move to trash' operation to also remove the note from trash as soon as it's flagged deleted and saved. That should trigger the lock on a multi-processor device, and without the lock the note is removed on the server without the deleted flag being toggled.

@roundhill
Copy link
Copy Markdown
Contributor

:shipit:

roundhill added a commit that referenced this pull request Apr 20, 2015
Issue #159: Send final modifications before remove operation
@roundhill roundhill merged commit b0aebdc into Simperium:develop Apr 20, 2015
@aforcier aforcier deleted the issue/159-concurrency-exception branch April 20, 2015 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Send final modifications before removing an object from a bucket

2 participants