levelup / palabre-extensions

Apache License 2.0
27 stars 3 forks source link

Crash when getting all articles from provider #2

Open revanmj opened 8 years ago

revanmj commented 8 years ago

Could you help me find the cause of the crash? One of my users is having a crash when getting all articles via Article.getAll (it works fine two times when processing read and unread articles, and after that when trying to process starred/saved ones this happens). Following error can be found in the logcat:

Fatal Exception: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean java.lang.Boolean.booleanValue()' on a null object reference at com.levelup.palabre.api.datamapping.Article.getAll(Article.java:359) at pl.revanmj.feedbinforpalabre.FeedbinExtension$7.onCompleted(FeedbinExtension.java:565) at pl.revanmj.feedbinforpalabre.FeedbinExtension$7.onCompleted(FeedbinExtension.java:559) at com.koushikdutta.async.future.SimpleFuture.handleCallbackUnlocked(SimpleFuture.java:107) at com.koushikdutta.async.future.SimpleFuture.setComplete(SimpleFuture.java:141) at com.koushikdutta.async.future.SimpleFuture.setComplete(SimpleFuture.java:128) at com.koushikdutta.ion.IonRequestBuilder$1.run(IonRequestBuilder.java:246) at com.koushikdutta.async.AsyncServer$RunnableWrapper.run(AsyncServer.java:57) at android.os.Handler.handleCallback(Handler.java:739) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:148) at android.app.ActivityThread.main(ActivityThread.java:5417) at java.lang.reflect.Method.invoke(Method.java) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)

I'm using 1.1.0 version of the Palabre library now, previously with 1.0.0 I also had similar crash but on com.levelup.palabre.api.datamapping.Article.title instead of getAll (and there was no line number).

PomepuyN commented 8 years ago

I just took a look at our code and this is a weird issue. Internally, we just iterate over a cursor and set the values to the Article. It acts as if the cursor was null but everything went fine a line before that. Are you processing Article data in multiple threads at this time?

revanmj commented 8 years ago

Yes, every step (processing read, unread, starred/saved) is a separate Ion task. So there may be a situation were more than one task is running at the same time.

PomepuyN commented 8 years ago

That may be the issue. I am not sure how the cursor will behave if the underlying data has been changed by another task. Are you able to reproduce the issue on your build? If so, could you try to synchronize the tasks to see if it fixes the issue?

revanmj commented 8 years ago

That's the problem - I was not able to reproduce the issue on my end so far and logs come from Crashlytics so I can't ask the user what exactly he/she did or what kind of articles are downloaded.

But thanks for a hint with threads, I'll try that and see if it fixes the issue.

PomepuyN commented 8 years ago

Ok, let me know if it works. Thanks for the report by the way.

revanmj commented 8 years ago

OK, now there is an issue with OOM on cursor when trying to get all articles.

Caused by java.lang.OutOfMemoryError: Failed to allocate a 34876 byte allocation with 5520 free bytes and 5KB until OOM.

android.database.CursorWindow.nativeGetString (CursorWindow.java)

android.database.CursorWindow.getString (CursorWindow.java:438)

android.database.AbstractWindowedCursor.getString (AbstractWindowedCursor.java:51)

android.database.CursorWrapper.getString (CursorWrapper.java:137)

android.database.CursorWrapper.getString (CursorWrapper.java:137)

com.levelup.palabre.provider.base.AbstractCursor.getStringOrNull (AbstractCursor.java:36)

com.levelup.palabre.provider.article.ArticleCursor.getContent (ArticleCursor.java:59)

com.levelup.palabre.api.datamapping.Article.getAll (Article.java:356)

revanmj commented 8 years ago

To give some context - I'm getting this list only to check if article with specific ID (this is all I get from Feedbin initially) is already in the database (if not, download it later) and to mark it as saved (or unread, but there is no crash in that part so far). So if there would be a function to do that without list of all articles, that would most likely also solve the problem.

revanmj commented 8 years ago

After working on this issue for some time I've found a workaround. I just used the following code to get a simple list with ids of all articles present in the DB. This is enough for my needs and so far seems to be small to enough to not throw OOM anywhere even when user has thousands of articles in the DB.

ArticleSelection articleSelection = new ArticleSelection(); ArticleCursor cursor = articleSelection.query(context.getContentResolver(), new String[] {ArticleColumns.FEEDLY_ID});

EDIT: And second one with date for onReadArticlesBefore method.

ArticleCursor cursor = articleSelection.query(context.getContentResolver(), new String[] {ArticleColumns.FEEDLY_ID, ArticleColumns.DATE});

I think those two are worth adding to the official API :)