sqlite3_close_v2 leading to database corruption

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

sqlite3_close_v2 leading to database corruption

Jens Alfke-2
We've been trying to figure out a database corruption issue for a week, and think we've got it figured out. It involves the "zombie database handle" state induced by sqlite3_close_v2:

> If sqlite3_close_v2() is called on a database connection that still has outstanding prepared statements, BLOB handles, and/orsqlite3_backup objects then it returns SQLITE_OK and the deallocation of resources is deferred until all prepared statements, BLOB handles, and sqlite3_backup objects are also destroyed.

In general this is useful behavior for us, as using Java bindings to our API can result in statement handles hanging around longer than they ought to until the JVM runs finalizers. With sqlite3_close_v2, closing the database doesn't fail in these circumstances. (Which is, I believe, why this function was added to SQLite in the first place.)

The situation where things go wrong is:
1. Database handle A is opened
2. A compiled statement is created and run. There's a Java object abstracting this sqlite3_stmt.
3. The database is closed with sqlite3_close_v2. The statement is still alive, so the database handle secretly remains open in "zombie mode".
4. We delete the database's parent directory and all files in it.
5. We create a new directory (with the same path as the old one) and call sqlite3_open_v2 to create a new empty database with handle B.
6. The JVM gets around to running finalizers, causing the sqlite3_stmt to be freed, which makes the zombie handle closable.
7. The internal function sqlite3LeaveMutexAndCloseZombie() closes the file handle … and also deletes the -wal and -shm files.

Now we're in trouble: the -wal and -shm files that just got deleted happen to belong to the *newly created* database, not the old one, so SQLite just nuked a live WAL. At this point the next SQLite call tends to return error 522, "disk I/O error", and the new db is unreadable.

This scenario isn't covered in "How To Corrupt An SQLite Database File", although what sqlite3LeaveMutexAndCloseZombie() does is similar to sections 1.3 or 2.4.

This sequence of events may look weird at this low level, but at a higher level they're not uncommon, especially in our own unit tests. The test suite is using our API to create a database (which we bundle in a directory), run a test on it, close it, then delete it. After that, the _next_ unit test does exactly the same thing, just with a different set of test functions. So the same database path ends up getting opened, closed, deleted, reopened over and over. (This pattern happens in real use too, not just in testing.)

At the moment I'm not sure what to do about this. The two options seem to be
(a) Stop using sqlite3_close_v2. This means we have to implement our own mechanism to forcibly close all open statements when requested to close a database, and clear the statement handles in the objects holding them.
(b) Avoid ever reusing the same path. This means we'd have to give the database file inside the wrapper directory a different name every time — instead of just "db.sqlite3" it'd include a UUID or timestamp or something. That of course complicates opening a database, necessitating a directory traversal to discover the file in the directory.

Anyone else run into this? Anyone agree it should be added to "How To Corrupt"? I don't know if it can be fixed inside SQLite...

—Jens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: sqlite3_close_v2 leading to database corruption

Simon Slavin-3
Before you call sqlite3_close_v2(), do this:

<https://sqlite.org/capi3ref.html#sqlite3_next_stmt>

and deal with any problems.

But actually I think you'll somehow have to force the JVM to run finalizers on the relevant objects.

Simon.
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: sqlite3_close_v2 leading to database corruption

Jens Alfke-2


> On Feb 6, 2018, at 4:13 PM, Simon Slavin <[hidden email]> wrote:
>
> Before you call sqlite3_close_v2(), do this:
> <https://sqlite.org/capi3ref.html#sqlite3_next_stmt <https://sqlite.org/capi3ref.html#sqlite3_next_stmt>>
> and deal with any problems.

Actually, I already do that, to log info about the open statements (see my earlier email today.)

But that doesn't directly help. This shows me the sqlite3_stmt pointers, but not the C++ objects of mine that own them. I'd need to find those and null out the statement pointers in them, which means I'd need a data structure that maps from a sqlite3_stmt to my object. This is basically the option (a) I sketched out.

There's the additional problem that sometimes there are open statements that don't belong to me, like the one in my earlier email that belongs to the FTS4 extension.

My current plan is to call sqlite3_close() first, then if it fails with SQLITE_BUSY I will call sqlite3_db_config() to enable SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE, and then call sqlite3_close_v2(). From my reading of sqlite3PagerClose() and sqlite3WalClose(), that config flag should prevent the WAL from being deleted when the zombie db closes.

—Jens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: sqlite3_close_v2 leading to database corruption

Clemens Ladisch
In reply to this post by Jens Alfke-2
Jens Alfke wrote:
> 3. The database is closed with sqlite3_close_v2. The statement is still alive, so the database handle secretly remains open in "zombie mode".
> 4. We delete the database's parent directory and all files in it.
> [...]
> This scenario isn't covered in "How To Corrupt An SQLite Database File"

sqlite3_close_v2() allows statements to be finalized afterwards, but
that finalization must actually happen for the DB to close.

You have a still-active database; this is described in 2.4.

> At the moment I'm not sure what to do about this.

Java's garbage collection handles memory.  For any other kind of
resource, you should use try-with-resources statements.

> (b) Avoid ever reusing the same path. This means we'd have to give the
>     database file inside the wrapper directory a different name every
>     time — instead of just "db.sqlite3" it'd include a UUID or
>     timestamp or something. That of course complicates opening
>     a database, necessitating a directory traversal to discover the
>     file in the directory.

That "wrapper directory" sounds as if you already have a mechanism to
run the test in a custom directory, so use a different directory name.


Regards,
Clemens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: sqlite3_close_v2 leading to database corruption

Dan Kennedy-4
In reply to this post by Jens Alfke-2
On 02/07/2018 07:25 AM, Jens Alfke wrote:

>
>> On Feb 6, 2018, at 4:13 PM, Simon Slavin <[hidden email]> wrote:
>>
>> Before you call sqlite3_close_v2(), do this:
>> <https://sqlite.org/capi3ref.html#sqlite3_next_stmt <https://sqlite.org/capi3ref.html#sqlite3_next_stmt>>
>> and deal with any problems.
> Actually, I already do that, to log info about the open statements (see my earlier email today.)
>
> But that doesn't directly help. This shows me the sqlite3_stmt pointers, but not the C++ objects of mine that own them. I'd need to find those and null out the statement pointers in them, which means I'd need a data structure that maps from a sqlite3_stmt to my object. This is basically the option (a) I sketched out.
>
> There's the additional problem that sometimes there are open statements that don't belong to me, like the one in my earlier email that belongs to the FTS4 extension.
>
> My current plan is to call sqlite3_close() first, then if it fails with SQLITE_BUSY I will call sqlite3_db_config() to enable SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE, and then call sqlite3_close_v2(). From my reading of sqlite3PagerClose() and sqlite3WalClose(), that config flag should prevent the WAL from being deleted when the zombie db closes.

That sounds like a pretty good workaround.

There is also now a patch on trunk to avoid this scenario:

   http://www.sqlite.org/src/info/4761db83b6d3d57f

Dan.

_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: sqlite3_close_v2 leading to database corruption

Jens Alfke-2
In reply to this post by Clemens Ladisch


> On Feb 7, 2018, at 12:34 AM, Clemens Ladisch <[hidden email]> wrote:
>
> Java's garbage collection handles memory.  For any other kind of
> resource, you should use try-with-resources statements.

Yes, like the way our .NET API uses IDisposable. But we can't _force_ developers to use try-with-resources, and we don't want an opportunity for a customer to corrupt databases because they forgot to use one, especially since it might take hours or days of time for our support engineers to figure out that's what they did.

> That "wrapper directory" sounds as if you already have a mechanism to
> run the test in a custom directory, so use a different directory name.

Sure, in our unit tests. But developers using our library can create and delete databases wherever they want, and it's not unheard-of for an app to delete a database and then create a replacement at the same path.

Anyway, looks like Dan has fixed the problem, so this is moot now!

—Jens
_______________________________________________
sqlite-users mailing list
[hidden email]
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users