Skip to content

Commit

Permalink
SQLite: Revert connection pool PR
Browse files Browse the repository at this point in the history 
Since merging, the CI fail rate increased significantly from ericsink/SQLitePCL.raw#430

Fixes dotnet#16202, unresolves dotnet#13837
  • Loading branch information
@bricelam
bricelam committed Aug 16, 2021
1 parent 52ecacb commit b8c69ed
Show file tree
Hide file tree
Showing 13 changed files with 104 additions and 932 deletions.
1 change: 0 additions & 1 deletion src/EFCore.Sqlite.Core/Storage/Internal/SqliteDatabaseCreator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ public override void Delete()

if (!string.IsNullOrEmpty(path))
{
SqliteConnection.ClearPool(new SqliteConnection(Dependencies.Connection.ConnectionString));
File.Delete(path);
}
}
Expand Down
7 changes: 2 additions & 5 deletions src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,8 @@ protected override DbConnection CreateDbConnection()
/// </summary>
public virtual ISqliteRelationalConnection CreateReadOnlyConnection()
{
var connectionStringBuilder = new SqliteConnectionStringBuilder(GetValidatedConnectionString())
{
Mode = SqliteOpenMode.ReadOnly,
Pooling = false
};
var connectionStringBuilder =
new SqliteConnectionStringBuilder(GetValidatedConnectionString()) { Mode = SqliteOpenMode.ReadOnly };

var contextOptions = new DbContextOptionsBuilder().UseSqlite(connectionStringBuilder.ToString()).Options;

Expand Down
17 changes: 11 additions & 6 deletions src/Microsoft.Data.Sqlite.Core/SqliteCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Data.Sqlite.Properties;
Expand Down Expand Up @@ -470,15 +471,19 @@ private IEnumerable<sqlite3_stmt> PrepareAndEnumerateStatements(Stopwatch timer)
{
DisposePreparedStatements(disposing: false);

var byteCount = Encoding.UTF8.GetByteCount(_commandText);
var sql = new byte[byteCount + 1];
Encoding.UTF8.GetBytes(_commandText, 0, _commandText.Length, sql, 0);

int rc;
sqlite3_stmt stmt;
var tail = _commandText;
var start = 0;
do
{
timer.Start();

string nextTail;
while (IsBusy(rc = sqlite3_prepare_v2(_connection!.Handle, tail, out stmt, out nextTail)))
ReadOnlySpan<byte> tail;
while (IsBusy(rc = sqlite3_prepare_v2(_connection!.Handle, sql.AsSpan(start), out stmt, out tail)))
{
if (CommandTimeout != 0
&& timer.ElapsedMilliseconds >= CommandTimeout * 1000L)
Expand All @@ -490,14 +495,14 @@ private IEnumerable<sqlite3_stmt> PrepareAndEnumerateStatements(Stopwatch timer)
}

timer.Stop();
tail = nextTail;
start = sql.Length - tail.Length;

SqliteException.ThrowExceptionForRC(rc, _connection.Handle);

// Statement was empty, white space, or a comment
if (stmt.IsInvalid)
{
if (tail.Length != 0)
if (start < byteCount)
{
continue;
}
Expand All @@ -509,7 +514,7 @@ private IEnumerable<sqlite3_stmt> PrepareAndEnumerateStatements(Stopwatch timer)

yield return stmt;
}
while (tail.Length != 0);
while (start < byteCount);

_prepared = true;
}
Expand Down
156 changes: 86 additions & 70 deletions src/Microsoft.Data.Sqlite.Core/SqliteConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Data.Common;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using Microsoft.Data.Sqlite.Properties;
using Microsoft.Data.Sqlite.Utilities;
using SQLitePCL;
Expand All @@ -23,6 +24,8 @@ public partial class SqliteConnection : DbConnection
{
internal const string MainDatabaseName = "main";

private const string DataDirectoryMacro = "|DataDirectory|";

private readonly List<WeakReference<SqliteCommand>> _commands = new();

private Dictionary<string, (object? state, strdelegate_collation? collation)>? _collations;
Expand All @@ -36,7 +39,7 @@ public partial class SqliteConnection : DbConnection

private string _connectionString;
private ConnectionState _state;
private SqliteConnectionInternal? _innerConnection;
private sqlite3? _db;
private bool _extensionsEnabled;
private int? _defaultTimeout;

Expand Down Expand Up @@ -66,7 +69,7 @@ public SqliteConnection(string? connectionString)
/// <value>A handle to underlying database connection.</value>
/// <seealso href="https://docs.microsoft.com/dotnet/standard/data/sqlite/interop">Interoperability</seealso>
public virtual sqlite3? Handle
=> _innerConnection?.Handle;
=> _db;

/// <summary>
/// Gets or sets a string used to open the connection.
Expand All @@ -79,7 +82,7 @@ public override string ConnectionString
{
get => _connectionString;

[MemberNotNull(nameof(_connectionString), nameof(PoolGroup))]
[MemberNotNull(nameof(_connectionString), nameof(ConnectionOptions))]
set
{
if (State != ConnectionState.Closed)
Expand All @@ -88,15 +91,11 @@ public override string ConnectionString
}

_connectionString = value ?? string.Empty;

PoolGroup = SqliteConnectionFactory.Instance.GetPoolGroup(_connectionString);
ConnectionOptions = new SqliteConnectionStringBuilder(_connectionString);
}
}

internal SqliteConnectionPoolGroup PoolGroup { get; set; }

internal SqliteConnectionStringBuilder ConnectionOptions
=> PoolGroup.ConnectionOptions;
internal SqliteConnectionStringBuilder ConnectionOptions { get; private set; }

/// <summary>
/// Gets the name of the current database. Always 'main'.
Expand Down Expand Up @@ -163,21 +162,6 @@ protected override DbProviderFactory DbProviderFactory
/// <value>The transaction currently being used by the connection.</value>
protected internal virtual SqliteTransaction? Transaction { get; set; }

/// <summary>
/// Empties the connection pool.
/// </summary>
/// <remarks>Any open connections will not be returned the the pool when closed.</remarks>
public static void ClearAllPools()
=> SqliteConnectionFactory.Instance.ClearPools();

/// <summary>
/// Empties the connection pool associated with the connection.
/// </summary>
/// <param name="connection">The connection.</param>
/// <remarks>Any open connections will not be returned the the pool when closed.</remarks>
public static void ClearPool(SqliteConnection connection)
=> connection.PoolGroup.Clear();

/// <summary>
/// Opens a connection to the database using the value of <see cref="ConnectionString" />. If
/// <c>Mode=ReadWriteCreate</c> is used (the default) the file is created, if it doesn't already exist.
Expand All @@ -190,9 +174,81 @@ public override void Open()
return;
}

_innerConnection = SqliteConnectionFactory.Instance.GetConnection(this);
var filename = ConnectionOptions.DataSource;
var flags = 0;

if (sqlite3_threadsafe() != 0)
{
flags |= SQLITE_OPEN_NOMUTEX;
}

if (filename.StartsWith("file:", StringComparison.OrdinalIgnoreCase))
{
flags |= SQLITE_OPEN_URI;
}

switch (ConnectionOptions.Mode)
{
case SqliteOpenMode.ReadOnly:
flags |= SQLITE_OPEN_READONLY;
break;

case SqliteOpenMode.ReadWrite:
flags |= SQLITE_OPEN_READWRITE;
break;

case SqliteOpenMode.Memory:
flags |= SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_MEMORY;
if ((flags & SQLITE_OPEN_URI) == 0)
{
flags |= SQLITE_OPEN_URI;
filename = "file:" + filename;
}

break;

default:
Debug.Assert(
ConnectionOptions.Mode == SqliteOpenMode.ReadWriteCreate,
"ConnectionOptions.Mode is not ReadWriteCreate");
flags |= SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
break;
}

switch (ConnectionOptions.Cache)
{
case SqliteCacheMode.Shared:
flags |= SQLITE_OPEN_SHAREDCACHE;
break;

case SqliteCacheMode.Private:
flags |= SQLITE_OPEN_PRIVATECACHE;
break;

default:
Debug.Assert(
ConnectionOptions.Cache == SqliteCacheMode.Default,
"ConnectionOptions.Cache is not Default.");
break;
}

var dataDirectory = AppDomain.CurrentDomain.GetData("DataDirectory") as string;
if (!string.IsNullOrEmpty(dataDirectory)
&& (flags & SQLITE_OPEN_URI) == 0
&& !filename.Equals(":memory:", StringComparison.OrdinalIgnoreCase))
{
if (filename.StartsWith(DataDirectoryMacro, StringComparison.InvariantCultureIgnoreCase))
{
filename = Path.Combine(dataDirectory, filename.Substring(DataDirectoryMacro.Length));
}
else if (!Path.IsPathRooted(filename))
{
filename = Path.Combine(dataDirectory, filename);
}
}

int rc;
var rc = sqlite3_open_v2(filename, out _db, flags, vfs: null);
SqliteException.ThrowExceptionForRC(rc, _db);

_state = ConnectionState.Open;
try
Expand Down Expand Up @@ -279,8 +335,8 @@ public override void Open()
}
catch
{
_innerConnection.Close();
_innerConnection = null;
_db.Dispose();
_db = null;

_state = ConnectionState.Closed;

Expand Down Expand Up @@ -318,53 +374,13 @@ public override void Close()

Debug.Assert(_commands.Count == 0);

_innerConnection!.Close();
_innerConnection = null;
_db!.Dispose();
_db = null;

_state = ConnectionState.Closed;
OnStateChange(new StateChangeEventArgs(ConnectionState.Open, ConnectionState.Closed));
}

internal void Deactivate()
{
int rc;

if (_collations != null)
{
foreach (var item in _collations.Keys)
{
rc = sqlite3_create_collation(Handle, item, null, null);
SqliteException.ThrowExceptionForRC(rc, Handle);
}
}

if (_functions != null)
{
foreach (var (name, arity) in _functions.Keys)
{
rc = sqlite3_create_function(Handle, name, arity, null, null);
SqliteException.ThrowExceptionForRC(rc, Handle);
}
}

if (_aggregates != null)
{
foreach (var (name, arity) in _aggregates.Keys)
{
rc = sqlite3_create_function(
Handle, name, arity, null, null, null);
SqliteException.ThrowExceptionForRC(rc, Handle);
}
}

// TODO: Unload extensions (currently not supported by SQLite)
if (_extensionsEnabled)
{
rc = sqlite3_enable_load_extension(Handle, 0);
SqliteException.ThrowExceptionForRC(rc, Handle);
}
}

/// <summary>
/// Releases any resources used by the connection and closes it.
/// </summary>
Expand Down
Loading

0 comments on commit b8c69ed

Please sign in to comment.