Concurrency crash

[Benoit Garret]

Tomdroid sometimes crashes with the following stacktrace:

Uncaught handler: thread main exiting due to uncaught exception
java.util.ConcurrentModificationException
at java.util.AbstractList$SimpleListIterator.next(AbstractList.java:66)
at org.tomdroid.NoteCollection.findNoteFromTitle(NoteCollection.java:71)
at org.tomdroid.ui.Tomdroid.updateNoteListWith(Tomdroid.java:284)
at org.tomdroid.ui.Tomdroid.access$0(Tomdroid.java:278)
at org.tomdroid.ui.Tomdroid$1.handleMessage(Tomdroid.java:243)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:123)
at android.app.ActivityThread.main(ActivityThread.java:3948)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:521)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:782)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:540)
at dalvik.system.NativeStart.main(Native Method)

I have attached a patch that should fix the crash. This is another quick fix as the notecollection will disappear.

[Olivier Bilodeau (plaxx)]

Was able to reproduce. I'll test if the fix do any performance regression. If there is no regression or it's not big i'll apply it shortly to main.

[Olivier Bilodeau (plaxx)]

Even with the fix, I was able to crash it again at another spot:

E/AndroidRuntime( 5121): java.lang.RuntimeException: Unable to start activity ComponentInfo{org.tomdroid/org.tomdroid.ui.ViewNote}: java.util.ConcurrentModificationException
E/AndroidRuntime( 5121): at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2141)
E/AndroidRuntime( 5121): at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2157)
E/AndroidRuntime( 5121): at android.app.ActivityThread.access$1800(ActivityThread.java:112)
E/AndroidRuntime( 5121): at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1581)
E/AndroidRuntime( 5121): at android.os.Handler.dispatchMessage(Handler.java:88)
E/AndroidRuntime( 5121): at android.os.Looper.loop(Looper.java:123)
E/AndroidRuntime( 5121): at android.app.ActivityThread.main(ActivityThread.java:3739)
E/AndroidRuntime( 5121): at java.lang.reflect.Method.invokeNative(Native Method)
E/AndroidRuntime( 5121): at java.lang.reflect.Method.invoke(Method.java:515)
E/AndroidRuntime( 5121): at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:739)
E/AndroidRuntime( 5121): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:497)
E/AndroidRuntime( 5121): at dalvik.system.NativeStart.main(Native Method)
E/AndroidRuntime( 5121): Caused by: java.util.ConcurrentModificationException
E/AndroidRuntime( 5121): at java.util.AbstractList$SimpleListIterator.next(AbstractList.java:59)
E/AndroidRuntime( 5121): at org.tomdroid.NoteCollection.buildNoteLinkifyPattern(NoteCollection.java:113)
E/AndroidRuntime( 5121): at org.tomdroid.ui.ViewNote.showNote(ViewNote.java:161)
E/AndroidRuntime( 5121): at org.tomdroid.ui.ViewNote.onCreate(ViewNote.java:102)
E/AndroidRuntime( 5121): at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1122)
E/AndroidRuntime( 5121): at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2104)
E/AndroidRuntime( 5121): ... 11 more

I'll see what I can do about this one.

[Benoit Garret (benoit.garret)]

I believe the most simple way to fix this would be to wrap all calls to notecollection with synchronized blocks. This should get us rid of the concurrent modifications issues.

The basic problem here is that the List used to store the notes in the NoceCollection is not thread safe.

[Olivier Bilodeau (plaxx)]

Like you said earlier, since we are getting rid of NoteCollection to the profit of a db that should handle all of this better, I'm not putting a lot of effort to make a clean fix.

Attached is my attempt at an ugly fix (warning: I'm no synchronization expert). Since ViewNote is also using findNoteFromTitle(...) I decided to make the method synchronized and dropped your patch.

I also synchronized buildNoteLinkifyPattern() and it did fix the crasher I experienced in comment #3.

I tested performance using the benchmark patch in doc/dev/threading-improvement/ and didn't notice any performance regression. Results added to benchmarks.txt

Can you test the patch and tell me if it fixes yours? I am no longer able to reproduce and not sure if I was ever able to..

[Olivier Bilodeau (plaxx)]

Ok, turns out I was able to reproduce your crash quite easily on the emulator. Just by reloading the app while hot (no reinstall) I got the same stacktrace as you did. On my ADP1, I was not able to reproduce your crash.

Anyways, the fix I proposed on comment #5 did work for me on both cases so I applied it to trunk/.

If you can try it and report back here it would be really appreciated since I want to release 0.2.0rc1 soon.

[Benoit Garret (benoit.garret)]

Can't seem to reproduce it with your fix, it's ok for me.

[Olivier Bilodeau (plaxx)]

fix released with 0.2 almost 7 months ago..