From 52eb23c43b1c9cf1e11e0a438f8cce17dafdbeec Mon Sep 17 00:00:00 2001 From: Elte Hupkes Date: Thu, 29 Aug 2019 14:04:39 +0200 Subject: [PATCH 1/2] Fix to #761, preventing incorrect disposed command object This addresses a race condition where a new PreparedSqliteInsertCommand object was returned in a disposed state if a second object was created and inserted into the command map during its creation. The fix always returns the item inserted into the command map. --- src/SQLite.cs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/SQLite.cs b/src/SQLite.cs index 0ce075e0..c1eac952 100644 --- a/src/SQLite.cs +++ b/src/SQLite.cs @@ -1710,21 +1710,20 @@ PreparedSqlLiteInsertCommand GetInsertCommand (TableMapping map, string extra) var key = Tuple.Create (map.MappedType.FullName, extra); lock (_insertCommandMap) { - _insertCommandMap.TryGetValue (key, out prepCmd); + if (_insertCommandMap.TryGetValue (key, out prepCmd)) { + return prepCmd; + } } - if (prepCmd == null) { - prepCmd = CreateInsertCommand (map, extra); - var added = false; - lock (_insertCommandMap) { - if (!_insertCommandMap.ContainsKey (key)) { - _insertCommandMap.Add (key, prepCmd); - added = true; - } - } - if (!added) { + prepCmd = CreateInsertCommand (map, extra); + + lock (_insertCommandMap) { + if (_insertCommandMap.TryGetValue (key, out var existing)) { prepCmd.Dispose (); + return existing; } + + _insertCommandMap.Add (key, prepCmd); } return prepCmd; From 3826756ffbb0185df8c38a45544b7a7066e6236f Mon Sep 17 00:00:00 2001 From: Elte Hupkes Date: Thu, 29 Aug 2019 14:40:57 +0200 Subject: [PATCH 2/2] Adding a regression test for #761 --- tests/ConcurrencyTest.cs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/ConcurrencyTest.cs b/tests/ConcurrencyTest.cs index 358a1133..ebf865bd 100644 --- a/tests/ConcurrencyTest.cs +++ b/tests/ConcurrencyTest.cs @@ -187,7 +187,28 @@ public void TestLoad() } } - + /// + /// Test for issue #761. Because the nature of this test is a race condition, + /// it is not guaranteed to fail if the issue is present. It does appear to + /// fail most of the time, though. + /// + [Test] + public void TestInsertCommandCreation () + { + using (var dbConnection = + new DbConnection (SQLiteOpenFlags.FullMutex | SQLiteOpenFlags.ReadWrite | SQLiteOpenFlags.Create)) { + var obj1 = new TestObj (); + var obj2 = new TestObj (); + var taskA = Task.Run (() => { + dbConnection.Insert (obj1); + }); + var taskB = Task.Run (() => { + dbConnection.Insert (obj2); + }); + + Task.WhenAll (taskA, taskB).Wait (); + } + } } }