Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SqliteConnection.Dispose does not release .db file on Windows (6.0 rc1 ->) #26369

Closed
erikbra opened this issue Oct 15, 2021 · 8 comments
Closed

Comments

@erikbra
Copy link

erikbra commented Oct 15, 2021

There seems to be a bug in Microsoft.Data.Sqlite, v 6.0, from rc1 onwards, which makes the database file remain locked after the SqliteConnection is closed, but only on Windows. It works up until preview7 (of the versions published on Nuget.org).

Repo with small repro can be found here: https://github.com/erikbra/Microsoft.Data.Sqlite-dispose-bug

    [TestFixture]
    public class SqliteDisposeTest
    {
        [Test]
        public async Task PureSqliteTest() {
            var file = "testdb.db";
            var connectionString = $"Data Source={file}";
            
            await using (var conn = new SqliteConnection(connectionString))
            {
                await conn.OpenAsync();

                await using (var cmd = conn.CreateCommand())
                {
                    cmd.CommandText = "CREATE TABLE jalla(name VARCHAR(40))";
                    await cmd.ExecuteNonQueryAsync();
                }
                File.Exists(file).Should().BeTrue();
            }
            
            // The line below (delete) works on versions up to and including 6.0.0-preview.7.21378.4, but fails on rc1 and rc2, on Windows (works on all versions on  Linux).
            File.Delete(file);
            File.Exists(file).Should().BeFalse();
        }
    }

It fails with the following stacktrace:

Failed PureSqliteTest [113 ms]
  Error Message:
   System.IO.IOException : The process cannot access the file 'C:\Users\erikb\Source\Repos\Microsoft.Data.Sqlite-dispose-bug\SimpleSqliteDisposeTest\bin\Debug\net6.0\testdb.db' because it is being used by another process.
  Stack Trace:
     at System.IO.FileSystem.DeleteFile(String fullPath)
   at System.IO.File.Delete(String path)
   at SimpleSqliteDisposeTest.SqliteDisposeTest.PureSqliteTest() in C:\Users\erikb\Source\Repos\Microsoft.Data.Sqlite-dispose-bug\SimpleSqliteDisposeTest\SqliteDisposeTest.cs:line 50
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.BlockUntilCompleted()
   at NUnit.Framework.Internal.MessagePumpStrategy.NoMessagePumpStrategy.WaitForCompletion(AwaitAdapter awaiter)
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.<>c__DisplayClass4_0.<PerformWork>b__0()
   at NUnit.Framework.Internal.ContextUtils.<>c__DisplayClass1_0`1.<DoIsolated>b__0(Object _)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated(ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated[T](Func`1 func)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.PerformWork()

Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1, Duration: 112 ms - SimpleSqliteDisposeTest.dll (net6.0)

Include version information

Microsoft.Data.Sqlite version: 6.0.0-rc.1.21452.10 and 6.0.0-rc.2.21480.5
Target framework: .NET 6.0rc2 (6.0.100-rc.2.21505.57)
Operating system: Windows 11, build 22478.1000), but probably on earlier versions too

@roji
Copy link
Member

roji commented Oct 15, 2021

This is by design - connection pooling was implemented in version 6.0 of the Sqlite provider, which keeps connections open so they can be recycled for better performance (see docs). You can use the SqliteConnection.ClearAllPools() API to force all idle pooled connections to be closed at a specific moment, or disable pooling altogether by adding Pooling=false to your connection string (and forgo the perf improvements).

@erikbra
Copy link
Author

erikbra commented Oct 16, 2021

Thanks a lot. I ran into a lot of issues with too many attached databases, etc, for my unit tests, so I had to resort to Pooling=false there: SQLite Error 1: 'too many attached databases - max 10'.

I'll look into it later, but turning off pooling makes it work as before, at least, Thanks!

@erikbra erikbra closed this as completed Oct 16, 2021
@erikbra
Copy link
Author

erikbra commented Oct 16, 2021

Just a follow-up question: Since the files were not locked on Linux, does that mean that connection pooling is not working in the same way on Linux, or just that file locking works differently on Linux fundamentally?

@ajcvickers
Copy link
Contributor

Re-opening to discuss with team the "SQLite Error 1: 'too many attached databases - max 10.'" error. Not sure how connection pooling would result in this.

/cc @bricelam

@ajcvickers ajcvickers reopened this Oct 16, 2021
@roji
Copy link
Member

roji commented Oct 16, 2021

Since the files were not locked on Linux, does that mean that connection pooling is not working in the same way on Linux, or just that file locking works differently on Linux fundamentally?

The latter - the moment you open a file under Windows (for writing IIRC), it is locked and other processes can no longer open it. Linux does not do that by default.

@roji
Copy link
Member

roji commented Oct 16, 2021

Thanks a lot. I ran into a lot of issues with too many attached databases, etc, for my unit tests, so I had to resort to Pooling=false there: SQLite Error 1: 'too many attached databases - max 10'.

Are you using the Sqlite ATTACH command in your application or tests? If so, can you please make sure you do DETACH as well? Pooling makes this necessary since connections get reused, effectively manifesting the attach "leak".

@erikbra
Copy link
Author

erikbra commented Oct 17, 2021

Hi again. Yes, I was indeed using "ATTACH", and was not "DETACH"-ing well enough. I'm developing a SQL took supporting many database types (SQL Server, PostgreSQL, MariaDB, etc), and want to keep things as similar as possible between the database types. The "CREATE DATABASE" and "DROP DATABASE" equivalents in Sqlite were a bit difficult to land on, but now I landed on:

  • The equivalent of CREATE DATABASE is just doing nothing
  • The equivalent of DROP DATABASE in Sqlite is deleting the file on disk (which now works, if I call SqliteConnection.ClearAllPools() first)

I've removed the usage of attach and detach, as they weren't really necessary for my use case.

Thanks for taking the time to answer, even though the error/misunderstanding was on my part!

@ajcvickers
Copy link
Contributor

@erikbra Thanks for gettin back to us.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants