chore: remove potential RPC calls in finalizer calls#771
Conversation
Remove potential RPC calls when finalizer calls are executed.
There was a problem hiding this comment.
Code Review
This pull request updates the Dispose(bool disposing) methods across several classes (AbstractLibObject, GrpcLibSpanner, and Server) to ensure that managed resources are only cleaned up when disposing is true, preventing finalizers from throwing exceptions or executing network operations. It also adds unit tests to verify this behavior. The review feedback correctly identifies that the new test Finalizer_DoesNotThrow_IfConnectionIsClosed is a false positive because closing the connection beforehand sets the disposed flag, causing the finalizer call to return early without executing the target logic; updating the test to run on an open connection is recommended.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the Dispose(bool disposing) methods in AbstractLibObject, GrpcLibSpanner, and Server to ensure that resource cleanup is only performed when disposing is true, preventing issues during finalization. It also adds unit tests to verify this behavior. The feedback suggests wrapping the Stop() call in Server.Dispose in a try-catch block to prevent exceptions from propagating during disposal, and explicitly specifying parameter types when using reflection in TransactionTests to avoid potential ambiguity.
6591f31 to
4e972e6
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the disposal and finalization logic across several classes, including AbstractLibObject, GrpcLibSpanner, and Server, to ensure that cleanup operations are only executed when explicitly disposing (disposing is true) rather than during finalization. It removes unnecessary finalizers and adds corresponding unit tests to verify that finalizers do not trigger these cleanup or rollback actions. There are no review comments to address, and I have no additional feedback to provide.
Remove potential RPC calls when finalizer calls are executed.