From 4ea9ecb1d1ec1ed6fa73f64ff156cdb2e2ffa638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 24 Jun 2026 09:16:34 +0200 Subject: [PATCH 1/4] chore: remove potential RPC calls in finalizer calls Remove potential RPC calls when finalizer calls are executed. --- .../spanner-ado-net-tests/ConnectionTests.cs | 19 ++++++++++++++++++- .../spanner-ado-net-tests/TransactionTests.cs | 17 +++++++++++++++++ .../SpannerLib/AbstractLibObject.cs | 2 +- .../SpannerLibGrpcImpl/GrpcLibSpanner.cs | 12 +++++++++--- .../SpannerLibGrpcServer/Server.cs | 7 +++++-- 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs b/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs index 4b197fca..28729806 100644 --- a/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs +++ b/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs @@ -555,5 +555,22 @@ public async Task ReadLargeString() var got = await conn!.ExecuteScalarAsync(sql); Assert.That(got, Is.EqualTo(value)); } - + + [Test] + public async Task Finalizer_DoesNotThrow_IfConnectionIsClosed() + { + var connection = new SpannerConnection(ConnectionString); + await connection.OpenAsync(); + + var libConnectionField = typeof(SpannerConnection).GetField("_libConnection", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + var libConnection = libConnectionField!.GetValue(connection); + + await connection.CloseAsync(); + + var disposeMethod = typeof(Google.Cloud.SpannerLib.AbstractLibObject).GetMethod("Dispose", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + + Assert.DoesNotThrow(() => disposeMethod!.Invoke(libConnection, new[] { (object)false })); + } } \ No newline at end of file diff --git a/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs b/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs index 74fabb41..aaf73932 100644 --- a/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs +++ b/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs @@ -850,4 +850,21 @@ public async Task CommitAsync_PropagatesException_IfConnectionIsClosed() Assert.ThrowsAsync(async () => await transaction.CommitAsync()); } + + [Test] + public async Task Finalizer_DoesNotExecuteRollback() + { + await using var connection = new SpannerConnection(ConnectionString); + await connection.OpenAsync(); + var transaction = await connection.BeginTransactionAsync(); + + var initialRollbacks = Fixture.SpannerMock.Requests.OfType().Count(); + + var disposeMethod = typeof(SpannerTransaction).GetMethod("Dispose", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + disposeMethod!.Invoke(transaction, new[] { (object)false }); + + var currentRollbacks = Fixture.SpannerMock.Requests.OfType().Count(); + Assert.That(currentRollbacks, Is.EqualTo(initialRollbacks)); + } } \ No newline at end of file diff --git a/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs b/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs index 2cc46e35..dfb6cf20 100644 --- a/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs +++ b/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs @@ -78,7 +78,7 @@ protected virtual void Dispose(bool disposing) } try { - if (Id > 0) + if (disposing && Id > 0) { CloseLibObject(); } diff --git a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs index ff4577a3..8dff4d47 100644 --- a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs +++ b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs @@ -131,11 +131,17 @@ private void Dispose(bool disposing) } try { - foreach (var channel in _channels) + if (disposing) { - channel.Dispose(); + if (_channels != null) + { + foreach (var channel in _channels) + { + channel?.Dispose(); + } + } + _server?.Dispose(); } - _server.Dispose(); } finally { diff --git a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs index 343d7316..3cd89d58 100644 --- a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs +++ b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs @@ -243,8 +243,11 @@ protected virtual void Dispose(bool disposing) } try { - Stop(); - _process?.Dispose(); + if (disposing) + { + Stop(); + _process?.Dispose(); + } } finally { From f18fd9ad50faee9c01ad5d49924739927bd39baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 24 Jun 2026 09:29:09 +0200 Subject: [PATCH 2/4] test: fix false-positive in test --- .../spanner-ado-net-tests/ConnectionTests.cs | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs b/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs index 28729806..ea11c95b 100644 --- a/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs +++ b/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs @@ -556,21 +556,42 @@ public async Task ReadLargeString() Assert.That(got, Is.EqualTo(value)); } + public class TestLibObject : Google.Cloud.SpannerLib.AbstractLibObject + { + public bool CloseLibObjectCalled { get; private set; } + + public TestLibObject(long id) : base(null!, id) + { + } + + protected override void CloseLibObject() + { + CloseLibObjectCalled = true; + } + + public void CallDispose(bool disposing) + { + Dispose(disposing); + } + } + [Test] - public async Task Finalizer_DoesNotThrow_IfConnectionIsClosed() + public void Finalizer_DoesNotCallCloseLibObject() { - var connection = new SpannerConnection(ConnectionString); - await connection.OpenAsync(); - - var libConnectionField = typeof(SpannerConnection).GetField("_libConnection", - System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); - var libConnection = libConnectionField!.GetValue(connection); + var testObj = new TestLibObject(id: 42); + + testObj.CallDispose(disposing: false); - await connection.CloseAsync(); + Assert.That(testObj.CloseLibObjectCalled, Is.False); + } + + [Test] + public void Dispose_CallsCloseLibObject() + { + var testObj = new TestLibObject(id: 42); - var disposeMethod = typeof(Google.Cloud.SpannerLib.AbstractLibObject).GetMethod("Dispose", - System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + testObj.CallDispose(disposing: true); - Assert.DoesNotThrow(() => disposeMethod!.Invoke(libConnection, new[] { (object)false })); + Assert.That(testObj.CloseLibObjectCalled, Is.True); } } \ No newline at end of file From 31799de7c3731c4f499af85bd1bf84be252c57ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 24 Jun 2026 10:48:46 +0200 Subject: [PATCH 3/4] chore: catch and ignore exceptions when stopping server --- .../spanner-ado-net/SpannerLibGrpcServer/Server.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs index 3cd89d58..e21d4dc0 100644 --- a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs +++ b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcServer/Server.cs @@ -245,7 +245,14 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - Stop(); + try + { + Stop(); + } + catch (Exception) + { + // Ignore exceptions during shutdown/dispose + } _process?.Dispose(); } } From 4e972e608a3f4b5292b628edb864f9b96367c5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 24 Jun 2026 10:54:36 +0200 Subject: [PATCH 4/4] chore: remove unused finalizers --- spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs | 2 +- spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs | 5 ++++- .../spanner-ado-net/SpannerLib/AbstractLibObject.cs | 5 ----- .../spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs | 4 +--- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs b/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs index ea11c95b..3f1ba69e 100644 --- a/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs +++ b/spanner-ado-net/spanner-ado-net-tests/ConnectionTests.cs @@ -556,7 +556,7 @@ public async Task ReadLargeString() Assert.That(got, Is.EqualTo(value)); } - public class TestLibObject : Google.Cloud.SpannerLib.AbstractLibObject + private class TestLibObject : Google.Cloud.SpannerLib.AbstractLibObject { public bool CloseLibObjectCalled { get; private set; } diff --git a/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs b/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs index aaf73932..4d24aae2 100644 --- a/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs +++ b/spanner-ado-net/spanner-ado-net-tests/TransactionTests.cs @@ -861,7 +861,10 @@ public async Task Finalizer_DoesNotExecuteRollback() var initialRollbacks = Fixture.SpannerMock.Requests.OfType().Count(); var disposeMethod = typeof(SpannerTransaction).GetMethod("Dispose", - System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic, + binder: null, + types: new[] { typeof(bool) }, + modifiers: null); disposeMethod!.Invoke(transaction, new[] { (object)false }); var currentRollbacks = Fixture.SpannerMock.Requests.OfType().Count(); diff --git a/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs b/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs index dfb6cf20..42f8a53e 100644 --- a/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs +++ b/spanner-ado-net/spanner-ado-net/SpannerLib/AbstractLibObject.cs @@ -42,11 +42,6 @@ internal AbstractLibObject(ISpannerLib spanner, long id) Id = id; } - ~AbstractLibObject() - { - Dispose(false); - } - protected void MarkDisposed() { _disposed = true; diff --git a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs index 8dff4d47..743ffdf1 100644 --- a/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs +++ b/spanner-ado-net/spanner-ado-net/SpannerLibGrpcImpl/GrpcLibSpanner.cs @@ -114,9 +114,7 @@ public GrpcLibSpanner( _clients[i] = new V1.SpannerLib.SpannerLibClient(_channels[i]); } } - - ~GrpcLibSpanner() => Dispose(false); - + public void Dispose() { Dispose(true);