Merge pull request #42544 from CyrusNajmabadi/sqlConnectionAllocation

Be more resilient to exceptions happening in sqlconnection.

src/Workspaces/Core/Portable/Storage/SQLite/Interop/SqlConnection.cs

@@ -54,6 +54,10 @@ public static SqlConnection Create(IPersistentStorageFaultInjector faultInjector
         {
             faultInjector?.OnNewConnection();
 
+            // Allocate dictionary before doing any sqlite work.  That way if it throws
+            // we don't have to do any additional cleanup.
+            var queryToStatement = new Dictionary<string, SqlStatement>();
+
             // Use SQLITE_OPEN_NOMUTEX to enable multi-thread mode, where multiple connections can be used provided each
             // one is only used from a single thread at a time.
             // see https://sqlite.org/threadsafe.html for more detail
@@ -67,16 +71,25 @@ public static SqlConnection Create(IPersistentStorageFaultInjector faultInjector
 
             Contract.ThrowIfNull(handle);
 
-            raw.sqlite3_busy_timeout(handle, (int)TimeSpan.FromMinutes(1).TotalMilliseconds);
-
-            var queryToStatement = new Dictionary<string, SqlStatement>();
-            return new SqlConnection(handle, faultInjector, queryToStatement);
+            try
+            {
+                raw.sqlite3_busy_timeout(handle, (int)TimeSpan.FromMinutes(1).TotalMilliseconds);
+                return new SqlConnection(handle, faultInjector, queryToStatement);
+            }
+            catch
+            {
+                // If we failed to create connection, ensure that we still release the sqlite
+                // handle.
+                raw.sqlite3_close(handle);
+                throw;
+            }
         }
 
         private SqlConnection(sqlite3 handle, IPersistentStorageFaultInjector faultInjector, Dictionary<string, SqlStatement> queryToStatement)
         {
-            // This constructor avoids allocations since failure (e.g. OutOfMemoryException) would leave the object
-            // partially-constructed, and the finalizer would run later triggering a crash.
+            // This constructor avoids allocations since failure (e.g. OutOfMemoryException) would
+            // leave the object partially-constructed, and the finalizer would run later triggering
+            // a crash.
             _handle = handle;
             _faultInjector = faultInjector;
             _queryToStatement = queryToStatement;
@@ -99,7 +112,11 @@ internal void Close_OnlyForUseBySqlPersistentStorage()
             Contract.ThrowIfNull(_handle);
 
             // release all the cached statements we have.
-            foreach (var statement in _queryToStatement.Values)
+            //
+            // use the struct-enumerator of our dictionary to prevent any allocations here.  We
+            // don't want to risk an allocation causing an OOM which prevents executing the
+            // following cleanup code.
+            foreach (var (_, statement) in _queryToStatement)
             {
                 statement.Close_OnlyForUseBySqlConnection();
             }