[fix] Recursion causing stack overflow when disposing client/service

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Next Release
 
 - Corrects all API documentation link references to point to their new locations
+- Fix recursion during disposal causing stack overflow
 
 ## v6.7.2 (2024-08-16)
 

EasyPost.Tests/ClientTest.cs

@@ -70,6 +70,15 @@ public void TestThreadSafety()
 
         private const string FakeApikey = "fake_api_key";
 
+        [Fact]
+        public void TestClientDisposal()
+        {
+            Client client = new(new ClientConfiguration(FakeApikey));
+            client.Dispose();
+
+            // As long as this test doesn't throw an exception, it passes
+        }
+
         [Fact]
         public void TestBaseUrlOverride()
         {

EasyPost.Tests/HttpTests/ClientConfigurationTest.cs

@@ -8,6 +8,16 @@ public class ClientConfigurationTest
     {
         #region Tests
 
+        [Fact]
+        public void TestClientConfigurationDisposal()
+        {
+            ClientConfiguration configuration = new("not_a_real_api_key");
+
+            configuration.Dispose();
+
+            // As long as this test doesn't throw an exception, it passes
+        }
+
         [Fact]
         [Testing.Function]
         public void TestClientConfiguration()

EasyPost.Tests/HttpTests/RequestTest.cs

@@ -0,0 +1,23 @@
+using System.Collections.Generic;
+using EasyPost._base;
+using EasyPost.Http;
+using Xunit;
+
+namespace EasyPost.Tests.HttpTests;
+
+public class RequestTest
+{
+    #region Tests
+
+    [Fact]
+    public void TestRequestDisposal()
+    {
+        Request request = new("https://google.com", "not_a_real_endpoint", Method.Get, ApiVersion.V2, new Dictionary<string, object> { { "key", "value" } }, new Dictionary<string, string> { { "header_key", "header_value" } });
+
+        request.Dispose();
+
+        // As long as this test doesn't throw an exception, it passes
+    }
+
+    #endregion
+}

EasyPost.Tests/ServicesTests/ServiceTest.cs

@@ -5,6 +5,7 @@
 using EasyPost.Exceptions.General;
 using EasyPost.Http;
 using EasyPost.Models.API;
+using EasyPost.Services;
 using EasyPost.Tests._Utilities;
 using EasyPost.Tests._Utilities.Attributes;
 using EasyPost.Utilities.Internal.Attributes;
@@ -21,6 +22,17 @@ public ServiceTests() : base("base_service")
         {
         }
 
+        [Fact]
+        public void TestServiceDisposal()
+        {
+            Client client = new(new ClientConfiguration("not_a_real_api_key"));
+
+            AddressService addressService = client.Address;
+            addressService.Dispose();
+
+            // As long as this test doesn't throw an exception, it passes
+        }
+
         /// <summary>
         ///     This test confirms that the GetNextPage method works as expected.
         ///     This method is implemented on a per-service, but rather than testing in each service, we'll test it once here.

EasyPost/Client.cs

@@ -190,46 +190,42 @@ public Client(ClientConfiguration configuration)
         /// <inheritdoc cref="EasyPostClient.Dispose(bool)"/>
         protected override void Dispose(bool disposing)
         {
-            // ref: https://dzone.com/articles/when-and-how-to-use-dispose-and-finalize-in-c
-            // ref: https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1063#pseudo-code-example
-            if (disposing)
-            {
-                // Dispose managed state (managed objects).
-                // "disposing" inherently true when called from Dispose(), so don't need to pass it in.
-
-                // Dispose of the services
-                Address.Dispose();
-                ApiKey.Dispose();
-                Batch.Dispose();
-                Billing.Dispose();
-                CarrierAccount.Dispose();
-                CarrierMetadata.Dispose();
-                CarrierType.Dispose();
-                Claim.Dispose();
-                CustomsInfo.Dispose();
-                CustomsItem.Dispose();
-                EndShipper.Dispose();
-                Event.Dispose();
-                Insurance.Dispose();
-                Order.Dispose();
-                Parcel.Dispose();
-                Pickup.Dispose();
-                Rate.Dispose();
-                ReferralCustomer.Dispose();
-                Refund.Dispose();
-                Report.Dispose();
-                ScanForm.Dispose();
-                Shipment.Dispose();
-                SmartRate.Dispose();
-                Tracker.Dispose();
-                User.Dispose();
-                Webhook.Dispose();
-
-                // Dispose of the Beta client
-                Beta.Dispose();
-            }
-
-            // Free native resources (unmanaged objects) and override a finalizer below.
+            if (!disposing) return;
+
+            // Dispose managed state (managed objects)
+
+            // "disposing" inherently true when called from Dispose(), so don't need to pass it in.
+
+            // Attempt to dispose of the services (some may already be disposed)
+            Address.Dispose();
+            ApiKey.Dispose();
+            Batch.Dispose();
+            Billing.Dispose();
+            CarrierAccount.Dispose();
+            CarrierMetadata.Dispose();
+            CarrierType.Dispose();
+            Claim.Dispose();
+            CustomsInfo.Dispose();
+            CustomsItem.Dispose();
+            EndShipper.Dispose();
+            Event.Dispose();
+            Insurance.Dispose();
+            Order.Dispose();
+            Parcel.Dispose();
+            Pickup.Dispose();
+            Rate.Dispose();
+            ReferralCustomer.Dispose();
+            Refund.Dispose();
+            Report.Dispose();
+            ScanForm.Dispose();
+            Shipment.Dispose();
+            SmartRate.Dispose();
+            Tracker.Dispose();
+            User.Dispose();
+            Webhook.Dispose();
+
+            // Attempt to dispose of the Beta client (may already be disposed)
+            Beta.Dispose();
 
             // Dispose of the base client
             base.Dispose(disposing);

EasyPost/ClientConfiguration.cs

@@ -147,21 +147,18 @@ public void Dispose()
     /// <inheritdoc cref="EasyPostClient.Dispose(bool)"/>
     protected virtual void Dispose(bool disposing)
     {
-        if (_isDisposed) return;
+        if (!disposing || _isDisposed) return;
 
-        if (disposing)
-        {
-            // Dispose managed state (managed objects).
+        // Set the disposed flag to true before disposing of the object to avoid infinite loops
+        _isDisposed = true;
 
-            // dispose of the prepared HTTP client
-            PreparedHttpClient?.Dispose();
+        // Dispose managed state (managed objects)
 
-            // dispose of the user-provided HTTP client
-            CustomHttpClient?.Dispose();
-        }
+        // Attempt to dispose of the prepared HTTP client (may already be disposed)
+        PreparedHttpClient?.Dispose();
 
-        // Free native resources (unmanaged objects) and override a finalizer below.
-        _isDisposed = true;
+        // Attempt to dispose of the user-provided HTTP client (may already be disposed)
+        CustomHttpClient?.Dispose();
     }
 
     /// <summary>

EasyPost/Http/Request.cs

@@ -159,17 +159,15 @@ public void Dispose()
         /// <inheritdoc cref="EasyPostClient.Dispose(bool)"/>
         protected virtual void Dispose(bool disposing)
         {
-            if (_isDisposed) return;
-            if (disposing)
-            {
-                // Dispose managed state (managed objects).
-
-                // Dispose the request message
-                _requestMessage.Dispose();
-            }
+            if (!disposing || _isDisposed) return;
 
-            // Free native resources (unmanaged objects) and override a finalizer below.
+            // Set the disposed flag to true before disposing of the object to avoid infinite loops
             _isDisposed = true;
+
+            // Dispose managed state (managed objects)
+
+            // Dispose the request message
+            _requestMessage.Dispose();
         }
 
         /// <summary>

EasyPost/_base/EasyPostClient.cs

@@ -222,17 +222,15 @@ public void Dispose()
         /// <param name="disposing">Whether this object is being disposed.</param>
         protected virtual void Dispose(bool disposing)
         {
-            if (_isDisposed) return;
-            if (disposing)
-            {
-                // Dispose managed state (managed objects).
-
-                // Dispose the configuration
-                _configuration.Dispose();
-            }
+            if (!disposing || _isDisposed) return;
 
-            // Free native resources (unmanaged objects) and override a finalizer below.
+            // Set the disposed flag to true before disposing of the object to avoid infinite loops
             _isDisposed = true;
+
+            // Dispose managed state (managed objects)
+
+            // Dispose of the configuration
+            _configuration.Dispose();
         }
 
         /// <summary>

EasyPost/_base/EasyPostService.cs

@@ -66,17 +66,15 @@ public void Dispose()
         /// <inheritdoc cref="EasyPostClient.Dispose(bool)"/>
         protected virtual void Dispose(bool disposing)
         {
-            if (_isDisposed) return;
-            if (disposing)
-            {
-                // Dispose managed state (managed objects).
+            if (!disposing || _isDisposed) return;
 
-                // Dispose the client
-                Client.Dispose();
-            }
-
-            // Free native resources (unmanaged objects) and override a finalizer below.
+            // Set the disposed flag to true before disposing of the object to avoid infinite loops
             _isDisposed = true;
+
+            // Dispose managed state (managed objects)
+
+            // Don't dispose of the associated client here, as it may be shared among multiple services
+            // Client.Dispose();
         }
 
         /// <summary>