1092844 - Close execSQL() cursor to fix "Cursor finalized without being closed" warnings

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Data Providers, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: part-1-close-execSQL-cursor.patch

When I run a debug build of Fennec with a clean profile and Sync, adb logcat reports ~50 warnings about "Cursor finalized without being closed":

E/GeckoMatrixCursor( 9179): Cursor finalized without being closed
E/GeckoMatrixCursor( 9179): java.lang.RuntimeException: stack
E/GeckoMatrixCursor( 9179):     at org.mozilla.gecko.sqlite.MatrixBlobCursor.finalize(MatrixBlobCursor.java:347)
E/GeckoMatrixCursor( 9179):     at java.lang.Daemons$FinalizerDaemon.doFinalize(Daemons.java:187)
E/GeckoMatrixCursor( 9179):     at java.lang.Daemons$FinalizerDaemon.run(Daemons.java:170)
E/GeckoMatrixCursor( 9179):     at java.lang.Thread.run(Thread.java:841)

These unclosed cursors are from SQLiteBridge calling internalQuery() to execute (non-query) SQL statements without closing the returned cursor. This patch replaces some non-query uses of internalQuery() with execSQL() and then closes execSQL()'s cursor.

Here is a green Try run:

https://tbpl.mozilla.org/?tree=Try&rev=28598807bdaf

Comment 1

[Chris Peterson [:cpeterson]]

Attachment: part-2-log-cursor-allocation-stack.patch

Part 2: Log MatrixBlobCursor's allocation stack, instead of the finalization stack, with "Cursor finalized without being closed" warnings.

Tracking down the unclosed cursors was difficult because the "Cursor finalized without being closed" warnings logged the finalization stack trace, which is useless and always the same. This patch captures the allocation stack trace in MatrixBlobCursor's constructor in DEBUG builds, so the warning can log something useful.

Some other Fennec classes have finalizers that log similar warnings; perhaps this patch should morph (in this bug or a follow-up) into a more generic facility for logging finalizer warnings?

Comment 2

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

Comment on attachment 8515669 [details] [diff] [review]
part-1-close-execSQL-cursor.patch

Seems reasonable. TBPL logs don't seem to have any bad output.

Comment 3

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf90a2ce6a02
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b156eff26e7

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cf90a2ce6a02
https://hg.mozilla.org/mozilla-central/rev/6b156eff26e7