handles.cs: Interlocked CAS for sqlite3 extra slot

src/SQLitePCLRaw.core/handles.cs

@@ -28,6 +28,7 @@ namespace SQLitePCL
     using System;
     using System.Collections.Concurrent;
     using System.Runtime.InteropServices;
+    using System.Threading;
 
     public class sqlite3_backup : SafeHandle
     {
@@ -346,24 +347,29 @@ internal void remove_stmt(sqlite3_stmt s)
         public T GetOrCreateExtra<T>(Func<T> f)
             where T : class, IDisposable
         {
-            if (extra != null)
+            // Audit #7: atomic check-and-set so two threads racing the first hook registration cannot both create T and silently drop one container's callbacks/GCHandles.
+            var existing = Volatile.Read(ref extra);
+            if (existing != null)
             {
-                return (T)extra;
+                return (T)existing;
             }
-            else
+            var candidate = f();
+            var previous = Interlocked.CompareExchange(ref extra, candidate, null);
+            if (previous != null)
             {
-                var q = f();
-                extra = q;
-                return q;
+                candidate.Dispose();
+                return (T)previous;
             }
+            return candidate;
         }
 
         private void dispose_extra()
         {
-            if (extra != null)
+            // Audit #8: pair Interlocked.Exchange with GetOrCreateExtra's CAS so disposal and creation don't race on a half-disposed object.
+            var snapshot = Interlocked.Exchange(ref extra, null);
+            if (snapshot != null)
             {
-                extra.Dispose();
-                extra = null;
+                snapshot.Dispose();
             }
         }
 

src/common/tests_xunit.cs

@@ -349,6 +349,44 @@ public void test_prepare_tail_string()
             }
         }
 
+        // Audit #7: GetOrCreateExtra used to allow two threads racing the first
+        // call to both create T, with one quietly winning extra and the loser's
+        // instance silently leaking.  The patched CAS path guarantees a single
+        // shared instance is returned to all callers.
+        sealed class extra_marker : IDisposable
+        {
+            public bool disposed;
+            public void Dispose() { disposed = true; }
+        }
+
+        [Fact]
+        public void test_get_or_create_extra_returns_same_instance_under_contention()
+        {
+            using (var db = ugly.open(":memory:"))
+            {
+                var n_threads = Math.Max(4, Environment.ProcessorCount);
+                var instances = new extra_marker[n_threads];
+                var threads = new System.Threading.Thread[n_threads];
+                var start = new System.Threading.ManualResetEventSlim();
+                for (var i = 0; i < n_threads; i++)
+                {
+                    var idx = i;
+                    threads[i] = new System.Threading.Thread(() =>
+                    {
+                        start.Wait();
+                        instances[idx] = db.GetOrCreateExtra(() => new extra_marker());
+                    });
+                    threads[i].Start();
+                }
+                start.Set();
+                foreach (var t in threads) t.Join();
+
+                Assert.NotNull(instances[0]);
+                for (var i = 1; i < n_threads; i++)
+                {
+                    Assert.Same(instances[0], instances[i]);
+                }
+            }
         // Regression test for the ArgumentOutOfRangeException variant of the
         // long-standing prepare-race bug class (see issues #108, #321, #430,
         // #479, #588). The provider's span overload of sqlite3_prepare_v2