Skip to content

fix(spanner): clear transaction connection after completion #16052

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.

Sign up for GitHub

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

Open
alaricd wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(spanner): clear transaction connection after completion #16052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 49 additions & 0 deletions apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.Tests/SpannerTransactionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Google.Cloud.Spanner.V1.Internal.Logging;
using Google.Cloud.Spanner.V1.Tests;
using System;
using System.Threading.Tasks;
using Xunit;

namespace Google.Cloud.Spanner.Data.Tests;
Expand Down Expand Up @@ -47,6 +48,54 @@ public void MaxCommitDelay_DefaultsToNull()
Assert.Null(transaction.TransactionOptions.MaxCommitDelay);
}

[Fact]
public void Connection_IsNullAfterDispose()
{
SpannerClient spannerClientMock = SpannerClientHelpers.CreateMockClient(Logger.DefaultLogger);
spannerClientMock.SetupCreateSessionAsync();
SpannerConnection connection = SpannerCommandTests.BuildSpannerConnection(spannerClientMock);
SpannerTransaction transaction = connection.BeginTransaction();

Assert.Same(connection, transaction.Connection);

transaction.Dispose();

Assert.Null(transaction.Connection);
}
Comment on lines +52 to +64

@gemini-code-assist gemini-code-assist Bot Oct 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure full compliance with the ADO.NET specification, consider adding test cases to verify that Connection also returns null after the transaction is committed or rolled back, as the transaction is no longer valid after these operations.

@alaricd alaricd Oct 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added regression coverage for Connection returning null after commit and rollback.


[Fact]
public async Task Connection_IsNullAfterCommit()
{
SpannerClient spannerClientMock = SpannerClientHelpers.CreateMockClient(Logger.DefaultLogger);
spannerClientMock.SetupCreateSessionAsync().SetupExecuteStreamingSql().SetupCommitAsync();
SpannerConnection connection = SpannerCommandTests.BuildSpannerConnection(spannerClientMock);
SpannerTransaction transaction = connection.BeginTransaction();

using (var command = connection.CreateSelectCommand("SELECT 1"))
{
command.Transaction = transaction;
using var reader = await command.ExecuteReaderAsync();
Assert.True(reader.HasRows);
}

await transaction.CommitAsync();

Assert.Null(transaction.Connection);
}

[Fact]
public async Task Connection_IsNullAfterRollback()
{
SpannerClient spannerClientMock = SpannerClientHelpers.CreateMockClient(Logger.DefaultLogger);
spannerClientMock.SetupCreateSessionAsync().SetupRollbackAsync();
SpannerConnection connection = SpannerCommandTests.BuildSpannerConnection(spannerClientMock);
SpannerTransaction transaction = connection.BeginTransaction();

await transaction.RollbackAsync();

Assert.Null(transaction.Connection);
}

[Theory, MemberData(nameof(ValidMaxCommitDelayValues))]
public void MaxCommitDelay_Valid(TimeSpan? maxCommitDelay)
{
Expand Down
5 changes: 4 additions & 1 deletion apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data/SpannerTransaction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public sealed class SpannerTransaction : SpannerTransactionBase, ISpannerTransac
private readonly bool _isRetriable = false;
private int _disposed = 0;
private int _commited = 0;
private int _completed = 0;

// Flag indicating whether the transaction has executed at least one statement.
// The TransactionTag may no longer be set once the transaction has executed one
Expand Down Expand Up @@ -153,7 +154,7 @@ public int CommitTimeout
public TimestampBound TimestampBound => _creationOptions.TimestampBound ?? _creationOptions.TransactionId?.TimestampBound;

/// <inheritdoc />
protected override DbConnection DbConnection => SpannerConnection;
protected override DbConnection DbConnection => _disposed == 0 && _completed == 0 ? SpannerConnection : null;

/// <summary>
/// <see cref="SpannerConnection"/> associated with this transaction.
Expand Down Expand Up @@ -448,6 +449,7 @@ Task<IEnumerable<long>> ISpannerTransaction.ExecuteBatchDmlAsync(ExecuteBatchDml
settings => settings.CommitSettings, TransactionOptions.EffectiveCommitTimeout(SpannerConnection), cancellationToken);
var response = await _session.CommitAsync(request, callSettings).ConfigureAwait(false);
Interlocked.Exchange(ref _commited, 1);
Interlocked.Exchange(ref _completed, 1);
// We dispose of the SpannerTransaction to inmediately release the session to the pool when possible.
Dispose();
if (response.CommitTimestamp == null)
Expand Down Expand Up @@ -503,6 +505,7 @@ public override async Task RollbackAsync(CancellationToken cancellationToken = d
await ExecuteHelper.WithErrorTranslationAndProfiling(
() => _session.RollbackAsync(new RollbackRequest(), callSettings),
"SpannerTransaction.Rollback", SpannerConnection.Logger).ConfigureAwait(false);
Interlocked.Exchange(ref _completed, 1);
Dispose();
}

Expand Down
Loading