Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.Data.Sqlite: Cleanup when error occurs on dispose. #32605

Merged
merged 2 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/Microsoft.Data.Sqlite.Core/SqliteTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,14 @@ private void Complete()
{
if (IsolationLevel == IsolationLevel.ReadUncommitted)
{
_connection!.ExecuteNonQuery("PRAGMA read_uncommitted = 0;");
try
{
_connection!.ExecuteNonQuery("PRAGMA read_uncommitted = 0;");
}
catch
{
// Ignore failure attempting to clean up.
}
}

_connection!.Transaction = null;
Expand All @@ -223,13 +230,19 @@ private void Complete()

private void RollbackInternal()
{
if (!ExternalRollback)
try
{
sqlite3_rollback_hook(_connection!.Handle, null, null);
_connection.ExecuteNonQuery("ROLLBACK;");
if (!ExternalRollback)
{
sqlite3_rollback_hook(_connection!.Handle, null, null);
_connection.ExecuteNonQuery("ROLLBACK;");
}
}
finally
{
Complete();
}

Complete();
}

private void RollbackExternal(object userData)
Expand Down
140 changes: 140 additions & 0 deletions test/Microsoft.Data.Sqlite.Tests/SqliteTransactionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

using System;
using System.Data;
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Microsoft.Data.Sqlite.Properties;
using Xunit;
using static SQLitePCL.raw;
Expand All @@ -11,6 +13,144 @@ namespace Microsoft.Data.Sqlite;

public class SqliteTransactionTest
{
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task SqliteTransaction_Dispose_does_not_leave_orphaned_transaction(bool async) // Issue #25119
{
using var connection = new FakeConnection("Data Source=:memory:");

if (async)
{
await connection.OpenAsync();
}
else
{
connection.Open();
}

#if NET5_0_OR_GREATER
using var transaction = async ? await connection.BeginTransactionAsync() : connection.BeginTransaction();
#else
using var transaction = connection.BeginTransaction();
#endif

await AddNewTable("Table1");

connection.SimulateFailureOnRollback = true;

try
{
#if NET5_0_OR_GREATER
if (async)
{
await transaction.DisposeAsync();
}
else
{
transaction.Dispose();
}
#else
transaction.Dispose();
#endif

Assert.Fail();
}
catch (Exception)
{
// Expected to throw.
}

Assert.Null(connection.Transaction);

connection.SimulateFailureOnRollback = false;

#if NET5_0_OR_GREATER
using var transaction2 = async ? await connection.BeginTransactionAsync() : connection.BeginTransaction();
#else
using var transaction2 = connection.BeginTransaction();
#endif

await AddNewTable("Table2");

#if NET5_0_OR_GREATER
if (async)
{
await transaction2.DisposeAsync();
}
else
{
transaction2.Dispose();
}
#else
transaction2.Dispose();
#endif

Assert.Null(connection.Transaction);

async Task AddNewTable(string tableName)
{
using var command = connection.CreateCommand();
command.CommandText = $"CREATE TABLE {tableName} (ID INT PRIMARY KEY NOT NULL);";
_ = async ? await command.ExecuteNonQueryAsync() : command.ExecuteNonQuery();
}
}

private class FakeCommand : SqliteCommand
{
private readonly FakeConnection _connection;
private readonly SqliteCommand _realCommand;

public FakeCommand(FakeConnection connection, SqliteCommand realCommand)
{
_connection = connection;
_realCommand = realCommand;
}

public override int ExecuteNonQuery()
{
var result = _realCommand.ExecuteNonQuery();

if (_connection.SimulateFailureOnRollback && CommandText.Contains("ROLLBACK"))
{
throw new SqliteException("Simulated failure", 1);
}

return result;
}

[AllowNull]
public override string CommandText { get => _realCommand.CommandText; set => _realCommand.CommandText = value; }
public override int CommandTimeout { get => _realCommand.CommandTimeout; set => _realCommand.CommandTimeout = value; }
public override CommandType CommandType { get => _realCommand.CommandType; set => _realCommand.CommandType = value; }
public override bool DesignTimeVisible { get => _realCommand.DesignTimeVisible; set => _realCommand.DesignTimeVisible = value; }

public override UpdateRowSource UpdatedRowSource
{
get => _realCommand.UpdatedRowSource;
set => _realCommand.UpdatedRowSource = value;
}

public override void Cancel()
=> _realCommand.Cancel();

public override object? ExecuteScalar()
=> _realCommand.ExecuteScalar();

public override void Prepare()
=> _realCommand.Prepare();
}

private class FakeConnection(string connectionString) : SqliteConnection(connectionString)
{
public bool SimulateFailureOnRollback { get; set; }

public override SqliteCommand CreateCommand()
=> new FakeCommand(this, base.CreateCommand());

public new SqliteTransaction? Transaction => base.Transaction;
}

[Fact]
public void Ctor_sets_read_uncommitted()
{
Expand Down
Loading