729252 - crash [@ android.database.StaleDataException: Access closed cursor at android.database.AbstractWindowedCursor.checkPosition(AbstractWindowedCursor.java)]

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

This bug was filed from the Socorro interface and is 
report bp-51dcc55f-62b2-4380-a5d3-6d89a2120217 .
============================================================= 
android.database.StaleDataException: Access closed cursor
	at android.database.AbstractWindowedCursor.checkPosition(AbstractWindowedCursor.java:217)
	at android.database.AbstractWindowedCursor.getInt(AbstractWindowedCursor.java:84)
	at android.database.CursorWrapper.getInt(CursorWrapper.java:123)
	at android.database.CursorWrapper.getInt(CursorWrapper.java:123)
	at org.mozilla.gecko.AwesomeBar.onContextItemSelected(AwesomeBar.java:477)
	at android.app.Activity.onMenuItemSelected(Activity.java:2209)
	at com.android.internal.policy.impl.PhoneWindow$ContextMenuCallback.onMenuItemSelected(PhoneWindow.java:2827)
	at com.android.internal.view.menu.MenuItemImpl.invoke(MenuItemImpl.java:143)
	at com.android.internal.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:855)
	at com.android.internal.view.menu.MenuDialogHelper.onClick(MenuDialogHelper.java:137)
	at com.android.internal.app.AlertController$AlertParams$3.onItemClick(AlertController.java:878)
	at android.widget.AdapterView.performItemClick(AdapterView.java:284)
	at android.widget.ListView.performItemClick(ListView.java:3763)
	at android.widget.AbsListView$PerformClick.run(AbsListView.java:1936)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:123)
	at org.mozilla.gecko.GeckoApp$34.run(GeckoApp.java:1824)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:123)
	at android.app.ActivityThread.main(ActivityThread.java:3687)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:507)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:842)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:600)
	at dalvik.system.NativeStart.main(Native Method)

Build ID : 20120211031145

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

I wonder if the cursor is being updated because of a notify, and the cached cursor used in the context menu is stale? We should probably close the context menu if we attempt to update - OR - delay the update until after the menu is closed. I kinda like the latter option myself.

Brian, Wes: Does that sound like a valid theory?

Comment 2

[Brian Nicholson (:bnicholson)]

(In reply to Mark Finkle (:mfinkle) from comment #1)
> I wonder if the cursor is being updated because of a notify, and the cached
> cursor used in the context menu is stale? We should probably close the
> context menu if we attempt to update - OR - delay the update until after the
> menu is closed. I kinda like the latter option myself.
> 
> Brian, Wes: Does that sound like a valid theory?

Sounds correct to me. Rather than storing a direct reference to the Cursor, we could instead use a ContentQueryMap (http://developer.android.com/reference/android/content/ContentQueryMap.html), so that we can safely access it after the Cursor is closed.

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Java crash; nom-ing for blocking fennec

Comment 4

[Brian Nicholson (:bnicholson)]

Attachment: patch

Caches the cursor data rather than holding a reference to the cursor itself.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 601469 [details] [diff] [review]
patch


>+            if (!(list == findViewById(R.id.bookmarks_list) &&
>+                    cursor.getInt(cursor.getColumnIndexOrThrow(Bookmarks.IS_FOLDER)) == 1)) {
>+                mContextMenuSubject = new ContextMenuSubject(
>+                        cursor.getInt(cursor.getColumnIndexOrThrow(Bookmarks._ID)),
>+                        cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.URL)),
>+                        cursor.getBlob(cursor.getColumnIndexOrThrow(URLColumns.FAVICON)),
>+                        cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.TITLE)));
>             }

The indenting here is throwing my OCD into a tailspin.

Can we try:

>             if (!(list == findViewById(R.id.bookmarks_list) && cursor.getInt(cursor.getColumnIndexOrThrow(Bookmarks.IS_FOLDER)) == 1)) {
>                 mContextMenuSubject =
>                       new ContextMenuSubject(cursor.getInt(cursor.getColumnIndexOrThrow(Bookmarks._ID)),
>                                              cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.URL)),
>                                              cursor.getBlob(cursor.getColumnIndexOrThrow(URLColumns.FAVICON)),
>                                              cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.TITLE))
>                       );
>             }

Comment 6

[Kevin Brosnan [Ex-Mozilla]]

Not a top crash, not blocking. Still good to fix though.

Comment 7

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/caf729dadb2f

Comment 8

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/caf729dadb2f