itkach / aard2-android

Aard2 for Android, a simple dictionary app
GNU General Public License v3.0
464 stars 98 forks source link

aard2 0.11 crashes frequently #10

Closed VolMi closed 9 years ago

VolMi commented 9 years ago

Hi, I'm getting a lot of crashes with version 0.11 wih log messages like

 E/AndroidRuntime(29060): java.lang.RuntimeException: Unable to start activity
ComponentInfo{itkach.aard2/itkach.aard2.ArticleCollectionActivity}:
java.lang.IllegalArgumentException: Comparison method violates its general contract!

First log, crash happened without interaction: http://pastebin.com/F66RSE4E

Second log, crash happened after clicking on an article name inside another article: http://pastebin.com/0a8YUGkN

itkach commented 9 years ago

Thank you for the report. This appears to be related to changed sorting behavior in Java 7 as described at http://www.oracle.com/technetwork/java/javase/compatibility-417013.html#source http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6804124

I am not seeing this crash, however, neither on Android 4.4.4 nor on Android 5. I also had a device running Cyanogen (Android 4.4.4, don't know exact build) for a few days, nothing crashed their either. I wonder if there's anything special about your Cyanogen build.

VolMi commented 9 years ago

I wonder if there's anything special about your Cyanogen build.

It is an official nightly for my S4 mini, super stable otherwise. I was using ART, which was always tagged as very experimental on 4.4. Now I switched back to Dalvik for testing, but the same crashes occur. I'm not aware of any other speciality of my build.

itkach commented 9 years ago

I was running on ART for some time and now it's default on 5.0. No crashes, so it's probably unrelated. It may very well be an issue in the code (I will look into it), it's just very odd that it doesn't happen for me on any of my devices.

itkach commented 9 years ago

It looks like it should only be crashing when either bookmarks or history is sorted by title, not time, and probably only when some specific titles are present. Further, since history moves pretty fast (both history and bookmarks keep only 100 most recently used items), it's probably something in your bookmarks. Can you clear history (long tap to select, select all using button in action bar) and toggle sort order on bookmarks to see if you can confirm?

itkach commented 9 years ago

As a side note, googling indicates that the aforementioned change in Java 7 (new sorting implementation that is unforgiving to broken comparators) caused countless crashes of countless programs and a lot of grief and unhappiness. In case of Aard 2, comparator in question uses ICU collation key comparison, and since Unicode collation is incredible complex there's probably an obscure bug in ICU where comparison is not transitive for some values. This decision to throw a runtime exception when broken comparator is detected was incredibly, unbelievably stupid idea. So now everybody is either switching to "legacy" implementation by setting a system property or writing their own sorting. Good job, Oracle!

VolMi commented 9 years ago

Hi itkach, thanks for digging. It seems you are right; I deleted my history and no crash occurred anymore. I've had only two bookmark entries, but with pure ASCII characters. Changing the sort mode there does not do any harm. Probably it was an entry in the history...?

itkach commented 9 years ago

Probably it was an entry in the history...?

Probably. And history was sorted by article title, right? In any case, it looks like the code either needs to catch and ignore exceptions from sort or stop using Java's standard sorting. There's a new version of ICU available, with new Unicode data and perhaps with some bugs fixed, but due to it's complexity there's always a chance that at least some collation key comparisons violate comparator contract.

itkach commented 9 years ago

b5cb8fc39671c395be086bf6255fc3fd458af4d7 should prevent crashes, although perhaps a better fix would be to use sorting that doesn't throw exceptions.