System.Data.SQLite

Login
This project makes use of Eagle, provided by Mistachkin Systems.
Eagle: Secure Software Automation
2024-04-15
22:00
Minor adjustments to the test for ticket [ce4d70ea6f]. check-in: abb207e382 user: mistachkin tags: trunk
2022-10-10
19:34
Minor adjustments to the test for ticket [ce4d70ea6f]. check-in: 5950fce56b user: mistachkin tags: trunk
2022-07-07
03:43 Ticket [ce4d70ea6f] SQLiteConnection does not appear to dispose of _sql field, might be a memory leak status still Pending with 3 other changes artifact: 13cbef8421 user: anonymous
00:19 Ticket [ce4d70ea6f]: 3 changes artifact: e87c3c3803 user: mistachkin
00:14
Minor corrections to the tests for ticket [ce4d70ea6f]. check-in: 0ee4656a4a user: mistachkin tags: trunk
2022-07-06
21:35 Pending ticket [ce4d70ea6f]: SQLiteConnection does not appear to dispose of _sql field, might be a memory leak plus 5 other changes artifact: 21d56e74ab user: mistachkin
21:33
Suppress finalizer calls for SQLite3 objects that are closed. Fix for [ce4d70ea6f]. check-in: e242effb07 user: mistachkin tags: trunk
15:09 Ticket [ce4d70ea6f] SQLiteConnection does not appear to dispose of _sql field, might be a memory leak status still Open with 3 other changes artifact: b5b3224b6f user: mistachkin
03:20 Ticket [ce4d70ea6f]: 3 changes artifact: f32335a6bd user: anonymous
00:48
Enhancements to the SQLiteConnection.Changed event. Cleanup several tests. Pursuant to [ce4d70ea6f]. check-in: 50a09e2598 user: mistachkin tags: trunk
2022-07-05
22:38 Ticket [ce4d70ea6f] SQLiteConnection does not appear to dispose of _sql field, might be a memory leak status still Open with 3 other changes artifact: 037fd01e02 user: mistachkin
16:27 Ticket [ce4d70ea6f]: 3 changes artifact: 8bf8402e8d user: mistachkin
09:02 Ticket [ce4d70ea6f]: 3 changes artifact: 8831407ab8 user: anonymous
01:32 Ticket [ce4d70ea6f]: 5 changes artifact: 19ed4a391c user: mistachkin
2022-07-04
08:55 Ticket [ce4d70ea6f]: 6 changes artifact: 113aeba7f2 user: anonymous
08:41 New ticket [ce4d70ea6f]. artifact: 1a0c07606c user: anonymous

Ticket Hash: ce4d70ea6f1ec2115a62dcf1cf5933fa32907943
Title: SQLiteConnection does not appear to dispose of _sql field, might be a memory leak
Status: Pending Type: Code_Defect
Severity: Important Priority: Medium
Subsystem: Connection Resolution: Fixed
Last Modified: 2022-07-07 03:43:12
Version Found In: 1.0.111
User Comments:
anonymous added on 2022-07-04 08:41:00:

SQLiteConnection has a internal SQLiteBase _sql field which holds a Sqlite3 object. Sqlite3 is IDisposable but SQLiteConnection but not appear to dispose of the _sql field when it is being disposed.

When many SQLiteConnections are being created, it appears that this can cause a performance issue (maybe a memory leak?). I profiled my application with dotMemory and noticed that there are many Sqlite3 objects in the finalizer queue.

The stacktrace in dotMemory looks like this.

System.Data.SQLite.SQLiteConnection.SetupSQLiteBase(SortedList<TKey, TValue>)
System.Data.SQLite.SQLiteConnection.Open()

My usage is like this:

using (var conn = new SQLiteConnection("Data Source=" + fullPathName + ";Version=3;" + (sharedMode ? "cache=shared" : string.Empty)))
{
    conn.Open();
    conn.Insert(row);
}

Could you check if the _sql field should indeed be disposed and if so, dispose it? Otherwise, would you know why I see many instances of Sqlite3 objects in the finalizer queue from dotMemory?


anonymous added on 2022-07-04 08:55:31:

btw, Looking at the source code of 1.0.116 (latest version), I see the the Dispose method on SQLiteConnection doesn't dispose of the _sql field too, so I expect it to have the same issue


mistachkin added on 2022-07-05 01:32:20:
Connection objects should be disposed.  It's a bit more complex than the
typical IDisposable pattern due to things like connection pooling.  This
behavior should be tested pretty thoroughly by the test suite; however,
just to be completely sure, I will double check the expected behavior
manually.

anonymous added on 2022-07-05 09:02:47:

I just attached an example program that reproduces what I saw, which is a very high number of finalized instances of Sqlite3 objects.

I'm expecting that they should be disposed and not appear in finalizable queue. If the GC isn't done in time, their memory consumption can grow significantly.


mistachkin added on 2022-07-05 16:27:30:
I looked at your example project and I strongly suspect is that Dapper is
somehow keeping the connection open past the end of the using block, e.g.
via not disposing of one or more SQLiteCommand and/or SQLiteDataReader
objects.

mistachkin added on 2022-07-05 22:38:56:

Using the following code of yours, slightly modified, appears to show the connections being properly closed. Aside: I have another variant of this without Dapper which also shows the connections being properly closed.

  using System.Data.SQLite;
  using System.Diagnostics;
  using System.Runtime.CompilerServices;
  using Dapper.Contrib.Extensions;

  string fullPathName = Path.Combine(
      AppDomain.CurrentDomain.BaseDirectory, "mydb.sqlite");

  const string tableScript = @"CREATE TABLE IF NOT EXISTS MyTable (
      [Id] INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
      Value INTEGER
      );";

  void CreateDB()
  {
      var fileInfo = new FileInfo(fullPathName);
      if (fileInfo.Exists)
      {
          fileInfo.Delete();
      }
      SQLiteConnection.CreateFile(fullPathName);
      using (var conn = new SQLiteConnection(
          "Data Source=" + fullPathName + ";Version=3;"))
      {
          conn.Open();
          var command = new SQLiteCommand(tableScript)
          {
              Connection = conn
          };
          using (command)
          {
              command.ExecuteNonQuery();
          }
      }
  }

  void SQLiteConnection_Changed(object sender, ConnectionEventArgs e)
  {
      if ((e.EventType == SQLiteConnectionEventType.Opened) ||
          (e.EventType == SQLiteConnectionEventType.Closed))
      {
          Console.WriteLine(String.Format("{0} {1}",
              RuntimeHelpers.GetHashCode(sender), e.EventType));
      }
  }

  void Main()
  {
      SQLiteConnection.Changed += new SQLiteConnectionEventHandler(
          SQLiteConnection_Changed);

      CreateDB();

      var watch = Stopwatch.StartNew();
      for(var i=0; i<10000; i++)
      {
          using (var conn = new SQLiteConnection(
              "Data Source=" + fullPathName + ";Version=3;"))
          {
              conn.Open();
              conn.Insert(new MyTable(){Value = 1});
          }
      }
      watch.Stop();
      Console.WriteLine($"took {watch.ElapsedMilliseconds/1000.0}s");
  }

  Main();


anonymous added on 2022-07-06 03:20:55:

It's not about the connection closing though. It's about the Sqlite3/ SQLiteBase object in the _sql field in SQLiteConnection not being cleaned immediately.

I changed the inner loop to this (so 100k runs and dapper inserts is commented)

for(var i=0; i<100000; i++)
{
    using (var conn = new SQLiteConnection(
               "Data Source=" + fullPathName + ";Version=3;"))
    {
        conn.Open();
        //conn.Insert(new MyTable(){Value = 1});
    }
}

I still see a large number of Sqlite3 objects in the finalizer queue, which I think should not be there. They should be disposed of when SQLiteConnection is disposed (via the using statement). If Sqlite3 object is disposed, then there should be a GC.SuppressFinalize call that prevent it from being put into the finalizer queue.

In this example, the program can run fast enough to clear the finalize queue quickly, but in a large program will many other allocations, this will cause a performance issue.

I think the solution should be as simple as adding a _sql.Dispose() call when SQLiteConnection is disposed.


mistachkin added on 2022-07-06 15:09:36:
Thanks for the additional information.

The underlying (native) resources associated with a SQLiteConnection do
get disposed when the connection is closed unless connection pooling is
enabled.

It sounds like your primary concern is the finalizer queue.  I think your
idea of using the GC.SuppressFinalize method is correct.  The code should
be doing that when it gets done cleaning up the underlying CriticalHandle
and its associated native resources.

I will work on adding that and the necessary tests.

mistachkin added on 2022-07-06 21:35:34:
Thanks again for the report.

The fix for this is now on trunk, complete with tests.

anonymous added on 2022-07-07 03:43:12:

Thanks! It's good to know that!