Sqlite-net: CloseAsync keeps database file locked

Created on 29 Sep 2018  ·  12Comments  ·  Source: praeclarum/sqlite-net

Code like the following throws an exception:

c# SQLiteAsyncConnection db = <create connection>; <use database> db.CloseAsync().Wait(); File.Delete(<database file>);

Bug

Most helpful comment

I've had the bug occur in a Xamarin app with both iOS and WPF targets (haven't tried other context). I didn't try awaiting, rather than calling Wait. I doubt it would make a difference, but it doesn't really matter, because both ways need to work. By the time that the task close the database completes, the database needs to be closed. As it is, however, something still holds a lock on the database file.

Looking at the sqlite-net code, the only bug I saw was unrelated: SQLiteConnectionPool.CloseConnection attempts to call Close on entry, even though entry will be null if TryGetValue returns false.

Unfortunately, the actual problem wasn't clear. A workaround is to add the following code to the example before the File.Delete statement. But such hackery shouldn't be needed of course.
c# db = null; GC.Collect(); GC.WaitForPendingFinalizers();

All 12 comments

Could you provide more information and context please?
Have you tried awaiting instead of waiting CloseAsync?

I've had the bug occur in a Xamarin app with both iOS and WPF targets (haven't tried other context). I didn't try awaiting, rather than calling Wait. I doubt it would make a difference, but it doesn't really matter, because both ways need to work. By the time that the task close the database completes, the database needs to be closed. As it is, however, something still holds a lock on the database file.

Looking at the sqlite-net code, the only bug I saw was unrelated: SQLiteConnectionPool.CloseConnection attempts to call Close on entry, even though entry will be null if TryGetValue returns false.

Unfortunately, the actual problem wasn't clear. A workaround is to add the following code to the example before the File.Delete statement. But such hackery shouldn't be needed of course.
c# db = null; GC.Collect(); GC.WaitForPendingFinalizers();

Hi
In our app we are actually awaiting and the problem is the same.

Why do we delete the database?
The device, running the app, can be shared by multiple users. So the database must get deleted if the user logs out. The reason is to prevent other users to access the database directly (and to circumvent the risk of having a bug exposing data of the previous user).

By the way, the hack with GC.Collect... does solve the problem.

What is the exception?
Do you wait for other operations before waiting for close? - maybe long running query is executing?

await and Wait are not working the same and diferently interacts (blocks) with current thread.

We are creating DB and removing it after each unit test in our test suite without any issues.

The exception is a file-in-use sharing violation.

Whether CloseAsync should wait for other operations to complete before its Task completes is a separate topic. Regardless of the close policy (wait for pending operations or cancel them), when CloseAsync's Task completes, the database file must not longer be in use.

Based on the GC workaround, we can infer that the bug is that Dispose is not being called somewhere, and so it isn't until a finalizer is running that the file is closed. Unfortunately, a unit test can get (un)lucky and miss the bug if the necessary finalizer happens to run before it removes the database file.

Are you completely sure that you are trying to delete correct file? SQLite is creating several temp and journal files while working.

Have you tried to attach source code version instead of nugget?

After CloseAsync completes, the .db3-shm and .db3-wal files still exist. Attempting to delete the .db3 file causes this exception:

System.IO.IOException: 'The process cannot access the file 'path.db3' because it is being used by another process.'

After GC.Collect(); GC.WaitForPendingFinalizers();, .db3-shm and .db3-wal are gone and .db3 can be deleted. This is using the latest NuGet version. I haven't tried from source.

Besides an exception on deletion, I can also reproduce the problem by putting a breakpoint on the line after db.CloseAsync().Wait();. When that breakpoint is hit, .db3-shm and .db3-wal should be gone, but for me, they still exist.

Regarding the SQLite documentation those files are “normally removed when the last connection to the database closes.”

SQLiteAsyncConnection could open two connections (one for writing and one for reading) depending on the opening flags.

Could you share your opening flags?

We are accessing DB using source code version with some custom fix and tweaks and don’t observe this kind of problems. I can check tomorrow if we have any specific fix for dispose related issues.

I open the database with db = new SQLiteAsyncConnection(path) - no flags. Thanks for checking. I look forward to seeing what you find.

Looking into our code changes there is a bug in CloseAsync method that does not close read connection. So when you create SQLiteAsyncConnection using one parameter (with two defaults) path constructor it's using:

SQLiteOpenFlags.FullMutex | SQLiteOpenFlags.ReadWrite | SQLiteOpenFlags.Create

as opening flags. When using FullMutex it creates two connections -

public SQLiteAsyncConnection (string databasePath, SQLiteOpenFlags openFlags, bool storeDateTimeAsTicks = true, object key = null)
{
    _openFlags = openFlags;
    isFullMutex = _openFlags.HasFlag (SQLiteOpenFlags.FullMutex);
    _connectionString = new SQLiteConnectionString (databasePath, storeDateTimeAsTicks, key);
    if (isFullMutex)
        _fullMutexReadConnection = new SQLiteConnectionWithLock (_connectionString, openFlags) { SkipLock = true };
}

_connectionString will be used with first DB operation (which causes another bug when closing without use and mentioned by you at the beginning of the thread) and _fullMutexReadConnection that is being created immediately.

When you look into CloseAsync or rest of the code there is nothing that closes _fullMutexReadConnection so it will probably be disposed only by going out of scope so that is why GC trick works.

public Task CloseAsync ()
{
    return Task.Factory.StartNew (() => {
        SQLiteConnectionPool.Shared.CloseConnection (_connectionString, _openFlags);
    }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
}

What we did - simply added code that will close read connection plus possibility to close read connection only because every SQLiteAsyncConnection with FullMutex will be creating its own read connection for every instance (connectionString) but for write will use shared connection from pool SQLiteConnectionPool.

public Task CloseAsync (bool closeOnlyReadConnection = false)
{
    _fullMutexReadConnection?.Close();
    if(closeOnlyReadConnection)
    {
        //TODO: use it when .NET 4.6 will be avaiable
        //return Task.CompletedTask;
        return Task.FromResult(true);
    }
    return Task.Factory.StartNew (() => {
        SQLiteConnectionPool.Shared.CloseConnection (_connectionString, _openFlags);
    }, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
}

It's also worth to mention when you look into provided tests then for synchronous connections authors are using using blocks when creating connections.

using (var db = new SQLiteConnection (path, true)) {
    db.CreateTable<OrderLine> ();
}

I also strongly encourage to close async connections making sure that every SQLiteAsyncConnection object is using CloseAsync.

Thanks @breyed for reporting the problem and thanks @awattar for investigating. I'll try to repro and fix.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

newtoncw picture newtoncw  ·  7Comments

jimmccurdy picture jimmccurdy  ·  6Comments

georgepiva picture georgepiva  ·  5Comments

Freddixx picture Freddixx  ·  4Comments

yaliashkevich picture yaliashkevich  ·  3Comments