Closed Bug 1106347 Opened 5 years ago Closed 5 years ago

Crash while typing in the search bar - crash in java.lang.IllegalStateException: Couldn''t read row 0, col 0 from CursorWindow. Make sure the Cursor is initialized correctly before accessing data from it. at android.database.CursorWindow.nativeGetBlob(Na

Categories

(Firefox for Android :: Favicon Handling, defect, critical)

34 Branch
All
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 37
Tracking Status
firefox34 --- wontfix
firefox35 + fixed
firefox36 + fixed
firefox37 --- fixed
fennec 35+ ---

People

(Reporter: Agi, Assigned: rnewman)

References

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

Summary: crash in java.lang.IllegalStateException: Couldn''t read row 0, col 0 from CursorWindow. Make sure the Cursor is initialized correctly before accessing data from it. at android.database.CursorWindow.nativeGetBlob(Native Method) → Crash while typing in the search bar
Looks like a regression from Bug 1003911 ?

Java Stack Trace.
java.lang.IllegalStateException: Couldn't read row 0, col 0 from CursorWindow.  Make sure the Cursor is initialized correctly before accessing data from it.
	at android.database.CursorWindow.nativeGetBlob(Native Method)
	at android.database.CursorWindow.getBlob(CursorWindow.java:399)
	at android.database.AbstractWindowedCursor.getBlob(AbstractWindowedCursor.java:45)
	at android.database.CursorWrapper.getBlob(CursorWrapper.java:122)
	at org.mozilla.gecko.db.LocalBrowserDB.getFaviconForUrl(LocalBrowserDB.java:1117)
	at org.mozilla.gecko.db.BrowserDB.getFaviconForFaviconUrl(BrowserDB.java:207)
	at org.mozilla.gecko.favicons.LoadFaviconTask.doInBackground(LoadFaviconTask.java:429)
	at org.mozilla.gecko.favicons.LoadFaviconTask$1.run(LoadFaviconTask.java:328)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
	at java.lang.Thread.run(Thread.java:841)
Component: Search Activity → Favicon Handling
Summary: Crash while typing in the search bar → Crash while typing in the search bar - crash in java.lang.IllegalStateException: Couldn''t read row 0, col 0 from CursorWindow. Make sure the Cursor is initialized correctly before accessing data from it. at android.database.CursorWindow.nativeGetBlob(Na
Attached file bug1106347-logcat.txt
Here is the logcat from just before the crash
This crash is currently #18 on Fx34. I don't see it on Fx35 though, but that could be population size.

Giovanni - Do you have any custom search engines installed? Does the crash happen if you pan through the History pane?

Richard - The stack says LocalBrowserDB.getFaviconForUrl(LocalBrowserDB.java:1117), but line 1117 is not in getFaviconForUrl using the Fx34 source. getFaviconForUrl does have a call to getBlob() but it's wrapped in a try/catch.
Flags: needinfo?(rnewman)
Flags: needinfo?(agi.novanta)
> Giovanni - Do you have any custom search engines installed? 
No. I switched the default to Google (if that helps)

>Does the crash happen if you pan through the History pane?
I just tried scrolling down a bit in the History pane and it crashed, it's reproducible every time I try.

bp-29625f67-136a-4aa9-b841-1ce952141130
Flags: needinfo?(agi.novanta)
tracking-fennec: --- → ?
(In reply to Mark Finkle (:mfinkle) from comment #3)

> LocalBrowserDB.getFaviconForUrl(LocalBrowserDB.java:1117), but line 1117 is
> not in getFaviconForUrl using the Fx34 source. getFaviconForUrl does have a
> call to getBlob() but it's wrapped in a try/catch.

It's only a try-finally, but it explicitly makes sure that moveToFirst() returns true, which means the cursor isn't empty.

The only reason I have found for this occurring is if the blob being retrieved is more than 1MB:

http://stackoverflow.com/questions/12716859/retrieve-large-blob-from-android-sqlite-database

We should limit the size of stored icons, add a catch here, and in that catch we should try to discard the over-sized icon.
Flags: needinfo?(rnewman)
Blocks: 1108084
This should do the trick. Untested, but.
Attachment #8532688 - Flags: review?(mark.finkle)
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Comment on attachment 8532688 [details] [diff] [review]
Don't die when retrieving over-large favicons from the database. v1

Is the DB connection in a read-only state here, since we only used a query? Will using the delete cause a problem or am I over-thinking it?
Attachment #8532688 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #7)

> Is the DB connection in a read-only state here, since we only used a query?
> Will using the delete cause a problem or am I over-thinking it?

It's a good question. We're safe for three reasons:

* In all but very exceptional situations, read-only sqlite connections in Android are actually read-write.

* When BrowserProvider handles the delete, it'll retrieve a writable database connection:


    public int deleteInTransaction(Uri uri, String selection, String[] selectionArgs) {
        trace("Calling delete in transaction on URI: " + uri);
        final SQLiteDatabase db = getWritableDatabase(uri);


That getWD call can (theoretically) fail if the DB is in a read-only state and the connection can't be exchanged/upgraded for a writable one. If so, the delete will fail, but we won't fall over.

* If we *are* going to crash here, then we'd also crash as soon as the user did anything that would cause a write -- saving a favicon for a new page, writing a history visit, pruning history on exit, etc.
https://hg.mozilla.org/mozilla-central/rev/f7f9ddc26c2d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
[Tracking Requested - why for this release]: crash we have a fix for - need to verify that this crash resolves the issue
Uplift requests?
tracking-fennec: ? → 35+
Flags: needinfo?(rnewman)
Giovanni, did this make the bug go away for you?
Flags: needinfo?(rnewman) → needinfo?(agi.novanta)
Comment on attachment 8532688 [details] [diff] [review]
Don't die when retrieving over-large favicons from the database. v1

Approval Request Comment
[Feature/regressing bug #]:
  None; Android platform issue.

[User impact if declined]:
  Crashes if large favicons make it into the DB.

[Describe test coverage new/current, TBPL]:
  None.

[Risks and why]: 
  Low risk. try-catch with a safe deletion attempt to avoid repeated failures.

[String/UUID change made/needed]:
  None.
Attachment #8532688 - Flags: approval-mozilla-aurora?
Attachment #8532688 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
We can consider this for beta after further verification and bake.
I moved to Nightly since I filed this bug and I haven't seen the crash any more. Although, I can't reproduce it in Firefox 34 either now, so it could be just bad luck.
Flags: needinfo?(agi.novanta)
Please nominate for beta uplift before Mon Dec 22 so we can consider this.
Flags: needinfo?(rnewman)
I see no crashes with this signature in 36 or 37 within the last 7 days.

On the other hand, I see no crashes with this signature in 36.0a2 at all, so I don't know how useful that is!
Flags: needinfo?(rnewman)
Comment on attachment 8532688 [details] [diff] [review]
Don't die when retrieving over-large favicons from the database. v1

Approval Request Comment
[Feature/regressing bug #]:
  None. Android limitation.

[User impact if declined]:
  Frequent crashes if a large favicon somehow has made it into the DB.

[Describe test coverage new/current, TBPL]:
  None.

[Risks and why]:
  Relatively low risk. See earlier Aurora approval. Requesting approval for Beta because historically this has been a high-volume crash, and it's nasty if you're affected; we don't see any crashes right now on Aurora, but that's very likely just due to limited user base.
 
[String/UUID change made/needed]:
  None.
Attachment #8532688 - Flags: approval-mozilla-beta?
Attachment #8532688 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.