"Duplicate Key 38" Keeps happening

[Subnet-Masked]

Please use GitHub reactions 👍 to show that you are affected by the same issue. Please don't comment if you have no relevant information to add!

Describe the bug

To Reproduce
Steps to reproduce the behavior:

1. Open the app and use as normal
2. Don't use the app for a bit
3. Open the app again

Expected behavior
Notes sync and load like normal

Smartphone (please complete the following information):

• Nextcloud Notes-Version (android app): 3.4.10
• F-Droid or Play Store: Fdroid
• Android-Version: 11 OxygenOS
• Device: OnePlus 8T + 5G

Server

• Nextcloud version: 21.0.2
• Nextcloud Notes version (server app): 4.0.4

Stacktrace

App Version: 3.4.10
App Version Code: 3004010
App Flavor: fdroid

Files App Version Code: 30160190

---

OS Version: 4.19.110-perf+(2104120358)
OS API Level: 30
Device: OnePlus8TTMO
Manufacturer: OnePlus
Model (and Product): KB2007 (OnePlus8TTMO)

---

java.lang.IllegalStateException: Duplicate key 37
	at j$.util.stream.-$$Lambda$Collectors$JCCUtBO-OYI_1WWo2UbajAktswI.apply(lambda:2)
	at j$.util.Map$-CC.$default$merge(Map.java:4)
	at j$.util.Map$-EL.merge(Map.java:9)
	at j$.util.stream.-$$Lambda$Collectors$vfQF-JJDCen_YKtRfgvkyywVMHc.accept(lambda:4)
	at j$.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:1)
	at j$.util.Iterator$-CC.$default$forEachRemaining(Iterator.java:3)
	at j$.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:5)
	at j$.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:6)
	at j$.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:3)
	at j$.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:5)
	at j$.util.stream.ReferencePipeline.collect(ReferencePipeline.java:17)
	at it.niedermann.owncloud.notes.persistence.NotesRepository.getIdMap(NotesRepository.java:424)
	at it.niedermann.owncloud.notes.persistence.NotesServerSyncTask.pullRemoteChanges(NotesServerSyncTask.java:208)
	at it.niedermann.owncloud.notes.persistence.NotesServerSyncTask.run(NotesServerSyncTask.java:96)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:462)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
	at java.lang.Thread.run(Thread.java:923)

I have cleared the app's data a few times and have logged out and back in, that usually gets it working again, but this issue persists. Only started happening after we upgraded to NC 21 accidentally because the updater said we were upgrading to 20.0.9

[stefan-niedermann]

sigh.... i believe we have a race condition in the latest releases, which causes multiple sync processes to run parallel and eventually inserting the same fetched note multiple times into the database of the Notes Android app. Your upgrade to NC 21 is coincidence and does not cause this error.

@korelstar maybe, if you have some time, could you try to help me looking for the cause here? I already tried to ensure that no syncs happen in parallel by making the NotesRepository#scheduleSync method synchronized and checking for already running syncs, but apparently this was not enough...

[korelstar]

@stefan-niedermann Are you able to reproduce that multiple synchronizations are executed in parallel? Or is it just an assumption?

I had a quick look on the code and found that HashMap is used from different threads, although HashMap is not thread-safe. This could be a cause. You use it, e.g., for syncActive.

[stefan-niedermann]

It is an assumption, i haven't been able to reproduce the issue myself, but it was reported multiple times in the past weeks.
I use ConcurrentHashMap already in a few place, i'll check the remaining HashMaps.

Though the syncActive and syncScheduled HashMaps are only called from within scheduleSync which is synchronized - so multithreading shouldn't be an issue there... 🤔

[stefan-niedermann]

@Prog1563 since you have experienced this issue already multiple times: Can you tell us more about the circumstances maybe? Did this happen after editing / adding a note or also when you only viewed the list?

[korelstar]

Though the syncActive and syncScheduled HashMaps are only called from within scheduleSync which is synchronized - so multithreading shouldn't be an issue there... 🤔

That's not true. You are using it in the inner anonymous class of type NotesServerSyncTask. That one is not synchronized to the same object. On the other hand, it's in onPreExecute/onPostExecute which should be executed on the UI thread.

[Subnet-Masked]

@Prog1563 since you have experienced this issue already multiple times: Can you tell us more about the circumstances maybe? Did this happen after editing / adding a note or also when you only viewed the list?

Unfortunately I don't have reliable steps to reproduce. I don't access my notes often but this seems to happen only after a while of leaving the app untouched. As for when it happens: Right after opening the app before trying to edit or view.

[stefan-niedermann]

You are using it in the inner anonymous class of type NotesServerSyncTask

While this is correct, the NotesServerSyncTask object is only supposed to live within the synchronized method. It gets created there, and all of its code is run completely sequentially. After its job is done, it should be destroyed - all before the synchronized scheduleSync method has been finished.

I changed the implementation from HashMap to ConcurrentHashMap for the mentioned maps, though i do not think that this will solve the problem. If a parallel write access occurs, it would have thrown a ConcurrentModificationException, and a read access from another app that doesn't conflict will still be successful, no?

[korelstar]

You are using it in the inner anonymous class of type NotesServerSyncTask

While this is correct, the NotesServerSyncTask object is only supposed to live within the synchronized method. It gets created there, and all of its code is run completely sequentially. After its job is done, it should be destroyed - all before the synchronized scheduleSync method has been finished.

That's not true. NotesServerSyncTask is a Runnable that is started with executor.submit(syncTask). The executor uses (background) threads for executing the task. This means, that scheduleSync() may has (and typically will be) finished before NotesServerSyncTask has finished. To be clear: The synchronized of scheduleSync() has no effects to the code in NotesServerSyncTask (and the derived anonymous class)!

I changed the implementation from HashMap to ConcurrentHashMap for the mentioned maps, though i do not think that this will solve the problem. If a parallel write access occurs, it would have thrown a ConcurrentModificationException, and a read access from another app that doesn't conflict will still be successful, no?

According to the docs, you can't rely on this:

Note that fail-fast behavior cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast operations throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: ConcurrentModificationException should be used only to detect bugs.

But nevertheless, I also think that it's unlikely, that this problem was the source. But maybe there are other places with wrong assumptions about synchronized or something similar.

[stefan-niedermann]

NotesServerSyncTask is a Runnable that is started with executor.submit(syncTask).

Oh man... how could i miss that?

Maybe using another Executor syncExecutor of type SingleThreadExecutor only for running NotesServerSyncTasks could solve this issue without the need of other synchronize keywords. This should make sure that at least no syncs run in parallel anymore. What do you think @korelstar ?

[stefan-niedermann]

@Prog1563 can you please update to 3.4.11 and let us know after some time whether this issue is still happening? We changed a few things and while i am not entirely sure that the issue should be fixed (since i am not entirely sure about the root cause yet) i need some feedback from you 🙂

[Subnet-Masked]

@stefan-niedermann Updating now from F-Droid. I'll let you know what happens :)

[daffydock]

I ended up having the same sort of issue. The problems comes, at least for me, I think, when I share content to a note. Then I switch back to another app I was in, to copy/paste more material.

As you mentioned, the Note app creates multiple notes every time I switched back and forth.

So, I went into my database and saw that I had thousands of File_lock entries. Put the instance into maintenance and deleted them all from the database.

Then, on the admin account, checked the logs, it gave me the exact files that were the culprits. Back on the user account, on the PC File app, I found the files in question, in my case there were about 5 or 6. Deleted those.

Clean cache on app, resynchronized and no more errors.

[stefan-niedermann]

@daffydock and can you please tell us if this happened on 3.4.11 or on only on earlier versions?

[daffydock]

I was on the previous Ver. But updated because I was hoping the update would clear it. So I did. It did not fix the issue, per sé.

Then I applied my fix and it worked. Have not tried going back and forth between two apps since. So, still no errors. Thus, I do not know if the update did anything -- alas, for perhaps stopping it from happening again? But I am not sure. Since the error was already there before I did the update. Also, for the time being, I have left my file.lock option on the Nextcloud config file as disabled. For now. Until I hear what others add to this topic.

[stefan-niedermann]

alas, for perhaps stopping it from happening again?

Exactly this.

The update tp 3.4.11 was not supposed to fix the issue if one already had it since the notes are locally duplicated we can not safely deterministically say which are the duplicates and which are original files. The attempt to "fix" this situation might lead to data loss and an even worse state. The crash and force to clear the storage if the app is therefore intented, even if i agree the it is not good from a UX perspective.

Version 3.4.11 is about avoiding further occurrences. When it is installed, the issue should not occur again (after clearing the storage if the issue was present).

[korelstar]

Maybe using another Executor syncExecutor of type SingleThreadExecutor only for running NotesServerSyncTasks could solve this issue without the need of other synchronize keywords. This should make sure that at least no syncs run in parallel anymore. What do you think @korelstar ?

Sorry for the late reply. I think using SingleThreadExecutor is fine and could help. Looking forward if this fixes the issue ...

[stefan-niedermann]

Closing this issue as two weeks have passed since the update and no further error has been reported so far.

Please feel free to reopen this issue or create a new one if the problem occurs again.