Ticket Hash: | aba45498017911a6d50e4791559a27cbc27c8c9c | ||
Title: | SQLiteStatementHandles are not being disposed properly with EF or Linq2SQL | ||
Status: | Closed | Type: | Feature_Request |
Severity: | Important | Priority: | Medium |
Subsystem: | Resource_Cleanup | Resolution: | Fixed |
Last Modified: | 2014-03-08 00:26:45 | ||
Version Found In: | 1.0.88.0 |
User Comments: | ||||
anonymous added on 2013-08-28 20:12:18:
When using EF+System.Data.SQLite or Linq2SQL+System.Data.SQLite some objects are not being disposed properly. I inserted line "KillSQLiteInterop(); GC.Collect();" after using{} to illustrate that. using System; using System.Data.Linq.Mapping; using System.IO; using System.Linq; using System.Runtime.InteropServices; Exception info: System.AccessViolationException was unhandled Message=Attempted to read or write protected memory. This is often an indication that other memory is corrupt. Source=System.Data.SQLite StackTrace: at System.Data.SQLite.UnsafeNativeMethods.sqlite3_finalize_interop(IntPtr stmt) at System.Data.SQLite.SQLiteBase.FinalizeStatement(SQLiteConnectionHandle hdl, IntPtr stmt) at System.Data.SQLite.SQLiteStatementHandle.ReleaseHandle() at System.Runtime.InteropServices.CriticalHandle.Cleanup() at System.Runtime.InteropServices.CriticalHandle.Dispose(Boolean disposing) at System.Runtime.InteropServices.CriticalHandle.Finalize() Disposing of SQLiteConnection should explicitely dispose all connected SQLiteConnectionHandles and SQLiteStatementHandles. If that will introduce some breaking changes, at least, please fire some events when objects with unmanaged part are being created - that will allow to manually dispose them when they are no longer needed. mistachkin added on 2013-08-29 03:41:46: Also see: http://www.mail-archive.com/sqlite-users@sqlite.org/msg67226.html mistachkin added on 2013-08-29 05:36:35: I suspect that LINQ is not disposing of the IDisposable objects that it creates internally to satisfy the query, in the following block of code: using (var ctx = new System.Data.Linq.DataContext(connMain)) { var insights = ctx.GetTable<Insights>(); var query = from ins in insights where ins.Pillar == "test 1" select ins; var list = query.ToList(); } anonymous added on 2013-08-29 10:10:04: Thanks for quick reply. Basically, yes, as SQLite + pure ADO.Net runs without any noticeable issues. I know about at least one issue with System.Data.Objects.ObjectContext.ExecuteStoreQueryInternal<TElement>. But I doubt if Microsoft will fix one of the .Net FrameWork core assembly because the same code with SQL CE provider runs without any issues. Most important - these objects are being created internally and developer has no control over it. As a temporary workaround for my project, I made a local copy of System.Data.SQLite code and added firing of a static event from SQLiteConnectionHandle(IntPtr db, bool ownHandle) and SQLiteStatementHandle(SQLiteConnectionHandle cnn, IntPtr stmt) constructors. This allows me to monitor such objects (along with SQLiteConnection and SQLiteCommand) and dispose them explicitely if they were not disposed after using statement. But local fork is not a best approach. Could you propose a more reliable solution which will be compatible with EF? mistachkin added on 2013-08-29 16:42:49: Changing to feature request. I'm going to add events to help monitor when any IDisposable objects are created. That way the developer can dispose of them even if the framework decides not to. mistachkin added on 2013-08-29 23:01:49: See check-in [85f27cc9f8]. The static OnChanged event for the connection should now be capable of notifying the application properly when public IDisposable objects are created. anonymous added on 2013-08-30 11:08:12: Thank you. I added my SQLiteResourceScope class to monitor these objects. I believe that solved most of issues for Linq2SLQ, but not for EF; for EF I still see the exception below and the DB file remains locked. System.AccessViolationException occurred Message=Attempted to read or write protected memory. This is often an indication that other memory is corrupt. Source=System.Data.SQLite StackTrace: at System.Data.SQLite.UnsafeNativeMethods.sqlite3_finalize_interop(IntPtr stmt) at System.Data.SQLite.SQLiteBase.FinalizeStatement(SQLiteConnectionHandle hdl, IntPtr stmt) in SQLiteBase.cs:line 672 Please find the example for EF and SQLiteResourceScope below. using System; using System.IO; using System.Linq; using System.Runtime.InteropServices; using System.Data.SQLite; using System.ComponentModel.DataAnnotations; using System.Data.Entity; using System.Data.Common; mistachkin added on 2013-08-31 08:36:31: Your example code seems reasonable. Is the exception still being thrown via the GC.Collect() method or somewhere else? mistachkin added on 2013-08-31 08:40:03: Actually, thinking more about this... I do not think your "resource scope" class should use WeakReference objects. Rather, it should store real references to all the IDisposable objects and then simply dispose of them upon its disposal. anonymous added on 2013-08-31 18:37:48: Yes, exception was thrown at GC.Collect: I'm using "KillSQLiteInterop(); GC.Collect();" construction to indicate what unmanaged objects were lost and why DB file remains locked. KillSQLiteInterop() method unloads SQLite.Interop.dll and thus makes impossible any further calls to unmanaged SQLite resourses. I'm using WeakReferences to let GC collect such objects - my intentions are just to force disposing of "lost" resources. If I will use direct references, this will hit badly memory management: in production application we need to create several 100MB databases from the scratch and therefore I must ensure that objects can be scraped as soon as they are no longer needed. As I said, as long as I'm additionally monitoring SQLiteConnectionHandles and SQLiteStatementHandles (and disposing them) - everything is fine. No sure if I should monitor SQLiteStatements as well. mistachkin added on 2013-09-03 04:52:50: A couple questions: 1. Does the issue reproduce if you use GC.Collect() prior to the "kill" call? 2. Does the problem reproduce if you use real references instead of weak ones? anonymous added on 2013-09-03 08:26:00: >1. Does the issue reproduce if you use GC.Collect() prior to the "kill" call? No, when I added "GC.Collect(); GC.WaitForPendingFinalizers(); GC.Collect();" prior to the KillSQLiteInterop() call I was not able to reproduce that exception (in my local dev environment, with low CPU load) 2. Does the problem reproduce if you use real references instead of weak ones? Yes, I changed type SQLiteResourceScope.Items to List<IDisposable> and I still observed the same exception. Here is statistics for the example above (with WeakReferences) 28 objects were listed Object added to the scope - exists (was not collected by GC) at the time of scope dispose:disposed state SQLiteCommand - no SQLiteDataReader - no SQLiteConnection - yes:True SQLiteCommand - no SQLiteDataReader - no SQLiteCommand - no SQLiteDataReader - no SQLiteCommand - yes:True SQLiteDataReader - yes:True SQLiteConnection - yes:False SQLiteCommand - yes:True SQLiteDataReader - yes:True SQLiteDataReader - yes:True SQLiteCommand - yes:True SQLiteDataReader - yes:True SQLiteCommand - yes:False SQLiteDataReader - yes:True SQLiteCommand - yes:True SQLiteDataReader - yes:True SQLiteCommand - yes:True SQLiteDataReader - yes:True SQLiteTransaction - yes:True SQLiteDataReader - yes:True SQLiteCommand - yes:True SQLiteDataReader - yes:True SQLiteCommand - yes:True SQLiteDataReader - yes:True SQLiteDataReader - yes:True anonymous added on 2013-09-03 11:02:21: BTW, here is statistics for the example with WeakReferences + SQLiteConnectionHandle / SQLiteStatementHandle / SQLiteStatement tracking added (it solves the problem). 64 objects were listed Object added to the scope - exists (was not collected by GC) at the time of scope dispose : disposed state SQLiteConnectionHandle - no SQLiteCommand - no SQLiteDataReader - no SQLiteStatementHandle - no SQLiteStatement - no SQLiteConnection - yes : true SQLiteCommand - no SQLiteDataReader - no SQLiteStatementHandle - no SQLiteStatement - no SQLiteCommand - no SQLiteDataReader - no SQLiteStatementHandle - no SQLiteStatement - no SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteConnection - yes : false SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : false SQLiteStatement - yes : false SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteCommand - yes : false SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteTransaction - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : false SQLiteStatement - yes : false SQLiteStatementHandle - yes : false SQLiteStatement - yes : false SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : false SQLiteStatement - yes : false mistachkin added on 2013-09-03 20:37:44: From your responses, it sounds like I may need to add the CriticalHandle derived classes to the OnChanged event so that they can be explicitly disposed as well? Would that completely solve the problem? mistachkin added on 2013-09-04 01:40:42: Another thing to try is to dispose of the remaining IDisposable objects in the following order: 1. All SQLiteDataReader objects. 2. All SQLiteCommand objects. 3. All SQLiteConnection objects. mistachkin added on 2013-09-04 06:52:34: See check-in [2843f57cae] on the "onChanged" branch. Does this code help to resolve the issue? anonymous added on 2013-09-04 12:05:58:
mistachkin added on 2013-09-04 01:40:42:Another thing to try is to dispose of the remaining IDisposable objects in the following order: I disposed all SQLiteDataReaders, then all SQLiteCommands, then all SQLiteConnections. As you can see, there are some SQLiteStatements/SQLiteStatementHandles remains undisposed. SQLiteConnectionHandle - no SQLiteCommand - no SQLiteDataReader - no SQLiteStatementHandle - no SQLiteStatement - no SQLiteConnection - yes : true SQLiteCommand - no SQLiteDataReader - no SQLiteStatementHandle - no SQLiteStatement - no SQLiteCommand - no SQLiteDataReader - no SQLiteStatementHandle - no SQLiteStatement - no SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteConnection - yes : true SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : false SQLiteStatement - yes : false SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteTransaction - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : false SQLiteStatement - yes : false SQLiteStatementHandle - yes : false SQLiteStatement - yes : false SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteConnectionHandle - yes : true SQLiteCommand - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : true SQLiteStatement - yes : true SQLiteDataReader - yes : true SQLiteStatementHandle - yes : false SQLiteStatement - yes : false anonymous added on 2013-09-04 12:28:45:
mistachkin added on 2013-09-04 06:52:34: Yes, that helps. BTW, could you additionally consider adding next lines to the SQLiteCommand.Dispose(bool disposing) method? if (_statementList != null) { lock (_statementList) { for (int i = 0; i < _statementList.Count; i++) _statementList[i].Dispose(); _statementList.Clear(); } _statementList = null; } mistachkin added on 2013-09-04 23:46:24: I'll try adding code similar to that; however, the disposal ordering for the SQLiteCommand/SQLiteDataReader stuff has been the source of issues before, when things were disposed too aggressively. mistachkin added on 2013-09-05 06:42:12: Further analysis reveals there would be no net impact when adding statement list disposal code to the SQLiteCommand.Dispose method. This is primarily due to how the disposal of the SQLiteCommand object ends up being deferred if there is an active SQLiteDataReader using it. anonymous added on 2013-09-05 07:40:53: Thanks a lot. I believe this will cover most of the issues with releasing of resources. mistachkin added on 2013-09-05 08:10:03: Please let me know if you are able to fully resolve this issue with the current System.Data.SQLite code on the onChanged branch. If so, I'll close this ticket as resolved fixed and merge the changes to trunk. anonymous added on 2013-09-05 10:35:46: Yes, it resolved this issue , thanks. mistachkin added on 2013-09-06 04:31:49: Changes checked-in on trunk [7e2f11fd49], including tests. anonymous added on 2014-02-27 20:57:59: I don't believe this fix is sufficient. I have the code, and am running into two problems: 1) CurrentInstance is never set to null, so creating a second SQLiteResourceScope is impossible. This is easily fixed by setting CurrentInstance = null in SQLiteResourceScope.Dispose. 2) After disposing a SQLiteResourceScope, I can no longer create a new instance of my DbContext and use it. It seems that Entity Framework caches DbCommand objects; I am getting an ObjectDisposedException when I try to query on the second object. Can somebody please look into this further? Thanks Joe Strommen joe.strommen@gmail.com public class TestContext : DbContext { public TestContext(DbConnection existingConnection) : base(existingConnection, false) { } public DbSet<TestEntity> TestEntities { get; set; } } public class TestEntity { public long Id { get; set; } public string Value { get; set; } } static void Main(string[] args) { var path = "test.sqlite"; if (File.Exists(path)) { File.Delete(path); } using (var sqliteConnection = new SQLiteConnection("Data Source=" + path)) { sqliteConnection.Open(); using (var sqliteCmd = sqliteConnection.CreateCommand()) { sqliteCmd.CommandText = "CREATE TABLE TestEntities (Id INTEGER PRIMARY KEY AUTOINCREMENT, Value TEXT);" + "INSERT INTO TestEntities ('Value') VALUES ('Value1'), ('Value2'), ('Value3');"; sqliteCmd.ExecuteNonQuery(); } using (new SQLiteResourceScope()) using (var efContext = new TestContext(sqliteConnection)) { var entityCount = efContext.TestEntities.Count(); } using (new SQLiteResourceScope()) // Problem #1 using (var efContext = new TestContext(sqliteConnection)) { var entityCount = efContext.TestEntities.Count(); // Problem #2 } } } mistachkin added on 2014-02-27 22:23:36: What is a SQLiteResourceScope? I do not see that class in the source code for System.Data.SQLite. mistachkin added on 2014-02-28 01:42:33: Related mailing list thread: http://www.mail-archive.com/sqlite-users%40sqlite.org/msg74770.html mistachkin added on 2014-02-28 01:44:12: FYI, just realized that the SQLiteResourceScope class is not part of the official System.Data.SQLite project. anonymous added on 2014-02-28 18:06:30: I was able to get this to work by building System.Data.SQLite with INTEROP_LEGACY_CLOSE enabled. It seems to me that until/unless the underlying issues get worked out, this should be the default. LINQ/EF6 support is pretty broken without it. mistachkin added on 2014-02-28 20:41:49: I think we'll have to agree to disagree on this point. Firstly, the issue is not with either SQLite nor System.Data.SQLite, but with the Entity Framework handling of IDisposable derived resources. Secondly, not being able to delete the underlying file for the database while the application is running is not really a critical feature. mistachkin added on 2014-03-06 06:45:22: I've attempted to reproduce this issue locally with Entity Framework 6 and the latest System.Data.SQLite release and everything seems to work properly. The issue may be with the SQLiteResourceScope class, which is not part of the System.Data.SQLite project and appears to no longer be necessary as of the Entity Framework 6 release. anonymous added on 2014-03-07 23:50:47: Being unable to delete the file is not my main concern - it's just the easiest-to-observe symptom of the resource leak that is occurring. The following code, using the most recent System.Data.SQLite NuGet package 1.0.91.3 (updated 3/7) still fails. class Program { public class TestContext : DbContext { public TestContext(DbConnection existingConnection) : base(existingConnection, false) { } public DbSet<TestEntity> TestEntities { get; set; } } public class TestEntity { public long Id { get; set; } public string Value { get; set; } } static void Main(string[] args) { var path = "test.sqlite"; if (File.Exists(path)) { File.Delete(path); } using (var sqliteConnection = new SQLiteConnection("Data Source=" + path)) { sqliteConnection.Open(); using (var sqliteCmd = sqliteConnection.CreateCommand()) { sqliteCmd.CommandText = "CREATE TABLE TestEntities (Id INTEGER PRIMARY KEY AUTOINCREMENT, Value TEXT);" + "INSERT INTO TestEntities ('Value') VALUES ('Value1'), ('Value2'), ('Value3');"; sqliteCmd.ExecuteNonQuery(); } using (var efContext = new TestContext(sqliteConnection)) { var entityCount = efContext.TestEntities.Count(); } } File.Delete(path); // Fails; the file is still locked. } } mistachkin added on 2014-03-08 00:26:45: I did a very careful analysis of the issue, in the VSdebugger, with resource creation hooks in place. The extra objects not being disposed are 100% for sure coming from inside the Entity Framework. I'm not sure how to force these objects to be disposed; however, that is the root cause of the issue. This is not a bug in System.Data.SQLite nor SQLite. |