Android Content Provider database leak issue

Question

I am writing a content provider for this application and in my content provider I am opening a database connection, running a query and returning the cursor of results to the calling program. If I close this database connection in the provider, the cursor has no results. If I leave it open, I get "leak found" errors in my DDMS log. What am I missing here? What's the clean, proper way to return a cursor of database results?

Winstead

It seems there is no need to close the database. The documentation says the database is cached so you will only have one database instance open for the entire application lifetime. developer.android.com/reference/android/database/sqlite/… – Naif

Answer 1

You're not missing anything AFAIK. Android is missing an onDestroy() (or the equivalent) for ContentProvider. There isn't even anything in the source code in this area that suggests there is some sort of onDestroy() that just isn't surfaced in the SDK.

If you look at the source code for AlarmProvider and LauncherProvider, they even create database objects on a per-API-call basis (e.g., every time they get insert(), they open a writable database handle that they never close).

Darrondarrow

argh that means I have to live with the open database :( – Pyroelectricity

"they even create database objects on a per-API-call basis [...] that they never close", see SQLiteOpenHelper#getWritableDatabase: "Once opened successfully, the database is cached". Though I agree, that one instance should be closed some time... – Rufe

Answer 2

One of my content providers manages multiple databases, same schema with different data sets. To prevent the IllegalStateException being through when garbage collection discovered that there was an open database in my content provider that no longer had anything referencing it, even though the SQLiteCursor does left me with several choices:

1) Leave the SQLiteDatabase object open and place it into a collection, never to be used again.

2) Leave the SQLiteDatabase object open and develop a cache that would allow me to reuse the database object when accessing the same database.

3) Close the database when the cursor is closed.

Solution 1 goes against my better judgement. Solution one is just another form of a resource leak; its one saving grace is that it does not upset the system. I ruled this choice out immediately.

Solution 2 was my idea of the best solution. It conserved resources at the same time reducing run-time by not having to reopen database connections. The down side of this solution was that I would have to write the cache and it would have increased the size of the application. Size really was not an issue but the time to write it was. I passed on this solution for the present time and may come back to it later.

Solution 3 is the one that decided to go with. First, I thought it would be simple to do; in the activity, all I needed to do was recast the Cursor returned by my Content Provider back into a SQLiteCursor. Then I could invoke its getDatabase() method and invoke close() on the database.

This is not possible. It turns out that the cursor returned from the Content Provider is within a wrapper class that prevents direct access to the actual Cursor object. The wrapper delegates method calls it receives to the Cursor. No chance to cast the cursor back to its derived type.

So, instead of placing the responsibility of closing the database on the activity, I went a different and easier route. I derived my own Cursor class by extending the SQLiteCursor class. I added two data members to my derived class; one to reference the database and the other was a ID for use in debugging. The class's ctor had the same signature as the SQLiteCursor ctor with an extra parameter added to the end for setting the ID value. The ctor set the database data member to insure that something was referencing the database if a garbage collection occurred before the cursor was closed.

I overrode the close() method so that it would both close the cursor [super.close()] and close the database if the reference was not null.

I also overrode the toString() method so that the ID number was appended to the string. This allowed me to track which cursors were still open and which ones have been opened and closed in the Log file.

I also added a method closeForReuse() so that the Content Provider could reuse a database for multiple queries without having to open a new database connection each time.

I also created a class that implemented the SQLiteDatabase.CursorFactory interface. It created my new derived cursor class and managed the ID value passed to each one.

This solution still requires that every activity closes the cursors passed to it when they are done using the cursor. Since this is good programming practice, it was not a concern.

Schaumberger

Answer 3

It is perfectly fine by leaving the database connection opened throughout the entire runtime of your app, you just have to make sure you close the cursor after once you're done with it.

I presume you're querying and using the cursors in an Activity? if so make sure you are closing the cursors by calling the cursor.close(); method, I notice if you're not closing the cursors in an Activity and then moving onto another Activity that you will get these leak messages when running another query.

I find that it's best practice to override the onDestroy method in your activity and close all the cursors in it.

Molly

I'm using the managedQuery call to call the provider. I was under the impression it handled the cursor lifecycle for me, but i'll go re-review just to make sure. Is it safe then to ignore these leak errors? – Winstead

I too made the same assumptions about the lifecycle, I started to notice the error messages disappearing once I started closing the cursors. At times I would get Exceptions thrown due to unclosed cursors when debugging, when that happened it killed my debug session, it almost never immediately throw these errors and Exceptions straight away. – Molly