Closed
Bug 1106347
Opened 9 years ago
Closed 9 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 Graveyard :: Favicon Handling, defect)
Tracking
(firefox34 wontfix, firefox35+ fixed, firefox36+ fixed, firefox37 fixed, fennec35+)
RESOLVED
FIXED
Firefox 37
People
(Reporter: agi, Assigned: rnewman)
References
Details
(Keywords: crash)
Crash Data
Attachments
(2 files)
11.97 KB,
text/plain
|
Details | |
2.06 KB,
patch
|
mfinkle
:
review+
Sylvestre
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
I have been getting a lot of crashes lately just typing in the search bar. It's related somehow to the search term because if I start typing the same thing when I restart the browser it crashes again. Reports: bp-08ab0239-5453-4d85-a70a-1adc72141130 bp-4747803c-e3ff-4844-8bd4-f12842141130 bp-ec15acd4-1cac-4f2c-a8d6-a7ae72141130 bp-1d093e1e-d529-4ac6-9697-7e6e02141129 bp-fa87f28a-69aa-48ee-b997-aec662141129 bp-8d095267-55c7-43ed-b73c-578772141129 bp-8c29691c-d42f-48ae-bebf-65d3a2141129 bp-5a3f446d-15a3-4879-964e-03b8d2141127 bp-7cc952ef-7cc1-4667-9142-7c31a2141127
Reporter | ||
Updated•9 years ago
|
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
Reporter | ||
Comment 1•9 years ago
|
||
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)
Reporter | ||
Updated•9 years ago
|
Component: Search Activity → Favicon Handling
Updated•9 years ago
|
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
Comment 2•9 years ago
|
||
Here is the logcat from just before the crash
Comment 3•9 years ago
|
||
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)
Reporter | ||
Comment 4•9 years ago
|
||
> 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)
Updated•9 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Comment 5•9 years ago
|
||
(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)
Assignee | ||
Comment 6•9 years ago
|
||
This should do the trick. Untested, but.
Attachment #8532688 -
Flags: review?(mark.finkle)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Comment 7•9 years ago
|
||
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+
Assignee | ||
Comment 8•9 years ago
|
||
(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.
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/f7f9ddc26c2d
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f7f9ddc26c2d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Comment 11•9 years ago
|
||
[Tracking Requested - why for this release]: crash we have a fix for - need to verify that this crash resolves the issue
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Flags: qe-verify?
Comment 12•9 years ago
|
||
Crash, tracking!
Assignee | ||
Comment 14•9 years ago
|
||
Giovanni, did this make the bug go away for you?
Flags: needinfo?(rnewman) → needinfo?(agi.novanta)
Assignee | ||
Comment 15•9 years ago
|
||
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?
Updated•9 years ago
|
Attachment #8532688 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 16•9 years ago
|
||
https://hg.mozilla.org/mozilla-aurora/rev/e9110e41fac6
Assignee | ||
Comment 17•9 years ago
|
||
We can consider this for beta after further verification and bake.
Reporter | ||
Comment 18•9 years ago
|
||
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)
Comment 19•9 years ago
|
||
Please nominate for beta uplift before Mon Dec 22 so we can consider this.
Flags: needinfo?(rnewman)
Assignee | ||
Comment 20•9 years ago
|
||
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)
Assignee | ||
Comment 21•9 years ago
|
||
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?
Updated•9 years ago
|
Attachment #8532688 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•