Fix memory leak in AWS library.

Author: JimBobSquarePants

src/ImageSharp.Web.Providers.AWS/AmazonS3BucketClient.cs

@@ -0,0 +1,56 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+using Amazon.S3;
+
+namespace SixLabors.ImageSharp.Web;
+
+/// <summary>
+/// Represents a scoped Amazon S3 client instance that is explicitly associated with a single S3 bucket.
+/// This wrapper provides a strongly-typed link between the client and the bucket it operates on,
+/// and optionally manages the lifetime of the underlying <see cref="AmazonS3Client"/>.
+/// </summary>
+public sealed class AmazonS3BucketClient : IDisposable
+{
+    private readonly bool disposeClient;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="AmazonS3BucketClient"/> class.
+    /// </summary>
+    /// <param name="bucketName">
+    /// The bucket name associated with this client instance.
+    /// </param>
+    /// <param name="client">
+    /// The underlying Amazon S3 client instance. This should be an already configured instance of <see cref="AmazonS3Client"/>.
+    /// </param>
+    /// <param name="disposeClient">
+    /// A value indicating whether the underlying client should be disposed when this instance is disposed.
+    /// </param>
+    public AmazonS3BucketClient(string bucketName, AmazonS3Client client, bool disposeClient = true)
+    {
+        Guard.NotNullOrWhiteSpace(bucketName, nameof(bucketName));
+        Guard.NotNull(client, nameof(client));
+        this.BucketName = bucketName;
+        this.Client = client;
+        this.disposeClient = disposeClient;
+    }
+
+    /// <summary>
+    /// Gets the bucket name associated with this client instance.
+    /// </summary>
+    public string BucketName { get; }
+
+    /// <summary>
+    /// Gets the underlying Amazon S3 client instance.
+    /// </summary>
+    public AmazonS3Client Client { get; }
+
+    /// <inheritdoc/>
+    public void Dispose()
+    {
+        if (this.disposeClient)
+        {
+            this.Client.Dispose();
+        }
+    }
+}

src/ImageSharp.Web.Providers.AWS/AmazonS3ClientFactory.cs

@@ -18,7 +18,7 @@ internal static class AmazonS3ClientFactory
     /// A new <see cref="AmazonS3Client"/>.
     /// </returns>
     /// <exception cref="ArgumentException">Invalid configuration.</exception>
-    public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options)
+    public static AmazonS3BucketClient CreateClient(IAWSS3BucketClientOptions options)
     {
         if (!string.IsNullOrWhiteSpace(options.Endpoint))
         {
@@ -27,7 +27,7 @@ public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options)
             // PathStyle endpoint doesn't support AccelerateEndpoint.
             AmazonS3Config config = new() { ServiceURL = options.Endpoint, ForcePathStyle = true, AuthenticationRegion = options.Region };
             SetTimeout(config, options.Timeout);
-            return new AmazonS3Client(options.AccessKey, options.AccessSecret, config);
+            return new(options.BucketName, new AmazonS3Client(options.AccessKey, options.AccessSecret, config));
         }
         else if (!string.IsNullOrWhiteSpace(options.AccessKey))
         {
@@ -36,14 +36,14 @@ public static AmazonS3Client CreateClient(IAWSS3BucketClientOptions options)
             RegionEndpoint region = RegionEndpoint.GetBySystemName(options.Region);
             AmazonS3Config config = new() { RegionEndpoint = region, UseAccelerateEndpoint = options.UseAccelerateEndpoint };
             SetTimeout(config, options.Timeout);
-            return new AmazonS3Client(options.AccessKey, options.AccessSecret, config);
+            return new(options.BucketName, new AmazonS3Client(options.AccessKey, options.AccessSecret, config));
         }
         else if (!string.IsNullOrWhiteSpace(options.Region))
         {
             RegionEndpoint region = RegionEndpoint.GetBySystemName(options.Region);
             AmazonS3Config config = new() { RegionEndpoint = region, UseAccelerateEndpoint = options.UseAccelerateEndpoint };
             SetTimeout(config, options.Timeout);
-            return new AmazonS3Client(config);
+            return new(options.BucketName, new AmazonS3Client(config));
         }
         else
         {

src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCache.cs

@@ -15,7 +15,7 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS;
 /// </summary>
 public class AWSS3StorageCache : IImageCache, IDisposable
 {
-    private readonly IAmazonS3 amazonS3Client;
+    private readonly AmazonS3BucketClient amazonS3Client;
     private readonly string bucketName;
     private readonly string cacheFolder;
     private bool isDisposed;
@@ -29,12 +29,13 @@ public AWSS3StorageCache(IOptions<AWSS3StorageCacheOptions> cacheOptions, IServi
     {
         Guard.NotNull(cacheOptions, nameof(cacheOptions));
         AWSS3StorageCacheOptions options = cacheOptions.Value;
-        this.bucketName = options.BucketName;
 
         this.amazonS3Client =
             options.S3ClientFactory?.Invoke(options, serviceProvider)
             ?? AmazonS3ClientFactory.CreateClient(options);
 
+        this.bucketName = this.amazonS3Client.BucketName;
+
         this.cacheFolder = string.IsNullOrEmpty(options.CacheFolder)
             ? string.Empty
             : options.CacheFolder.Trim().Trim('/') + '/';
@@ -48,8 +49,8 @@ public AWSS3StorageCache(IOptions<AWSS3StorageCacheOptions> cacheOptions, IServi
         try
         {
             // HEAD request throws a 404 if not found.
-            MetadataCollection metadata = (await this.amazonS3Client.GetObjectMetadataAsync(request)).Metadata;
-            return new AWSS3StorageCacheResolver(this.amazonS3Client, this.bucketName, keyWithFolder, metadata);
+            MetadataCollection metadata = (await this.amazonS3Client.Client.GetObjectMetadataAsync(request)).Metadata;
+            return new AWSS3StorageCacheResolver(this.amazonS3Client.Client, this.bucketName, keyWithFolder, metadata);
         }
         catch
         {
@@ -75,7 +76,7 @@ public Task SetAsync(string key, Stream stream, ImageCacheMetadata metadata)
             request.Metadata.Add(d.Key, d.Value);
         }
 
-        return this.amazonS3Client.PutObjectAsync(request);
+        return this.amazonS3Client.Client.PutObjectAsync(request);
     }
 
     /// <summary>
@@ -123,7 +124,8 @@ public void Dispose()
 
     private static async Task<PutBucketResponse?> CreateIfNotExistsAsync(AWSS3StorageCacheOptions options, S3CannedACL acl)
     {
-        AmazonS3Client client = AmazonS3ClientFactory.CreateClient(options);
+        using AmazonS3BucketClient bucketClient = AmazonS3ClientFactory.CreateClient(options);
+        AmazonS3Client client = bucketClient.Client;
 
         bool foundBucket = false;
         ListBucketsResponse listBucketsResponse = await client.ListBucketsAsync();

src/ImageSharp.Web.Providers.AWS/Caching/AWSS3StorageCacheOptions.cs

@@ -1,8 +1,6 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
-using Amazon.S3;
-
 namespace SixLabors.ImageSharp.Web.Caching.AWS;
 
 /// <summary>
@@ -11,7 +9,7 @@ namespace SixLabors.ImageSharp.Web.Caching.AWS;
 public class AWSS3StorageCacheOptions : IAWSS3BucketClientOptions
 {
     /// <inheritdoc/>
-    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientFactory { get; set; }
+    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3BucketClient>? S3ClientFactory { get; set; }
 
     /// <inheritdoc/>
     public string? Region { get; set; }

src/ImageSharp.Web.Providers.AWS/IAWSS3BucketClientOptions.cs

@@ -13,15 +13,16 @@ public interface IAWSS3BucketClientOptions
     /// <summary>
     /// Gets or sets a custom a factory method to create an <see cref="AmazonS3Client"/>.
     /// </summary>
-    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientFactory { get; set; }
+    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3BucketClient>? S3ClientFactory { get; set; }
 
     /// <summary>
     /// Gets or sets the AWS region endpoint (us-east-1/us-west-1/ap-southeast-2).
     /// </summary>
     public string? Region { get; set; }
 
     /// <summary>
-    /// Gets or sets the AWS bucket name. Cannot be <see langword="null"/>.
+    /// Gets or sets the AWS bucket name.
+    /// Cannot be <see langword="null"/> when <see cref="S3ClientFactory"/> is not set.
     /// </summary>
     public string BucketName { get; set; }
 

src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProvider.cs

@@ -24,7 +24,7 @@ public class AWSS3StorageImageProvider : IImageProvider, IDisposable
     /// <summary>
     /// The containers for the blob services.
     /// </summary>
-    private readonly Dictionary<string, AmazonS3Client> buckets
+    private readonly Dictionary<string, AmazonS3BucketClient> buckets
         = new();
 
     private readonly AWSS3StorageImageProviderOptions storageOptions;
@@ -55,11 +55,11 @@ public AWSS3StorageImageProvider(
 
         foreach (AWSS3BucketClientOptions bucket in this.storageOptions.S3Buckets)
         {
-            AmazonS3Client s3Client =
+            AmazonS3BucketClient s3Client =
                 bucket.S3ClientFactory?.Invoke(bucket, serviceProvider)
                 ?? AmazonS3ClientFactory.CreateClient(bucket);
 
-            this.buckets.Add(bucket.BucketName, s3Client);
+            this.buckets.Add(s3Client.BucketName, s3Client);
         }
     }
 
@@ -84,7 +84,7 @@ public bool IsValidRequest(HttpContext context)
         // the remaining path string as the key.
         // Path has already been correctly parsed before here.
         string bucketName = string.Empty;
-        IAmazonS3? s3Client = null;
+        AmazonS3Client? s3Client = null;
 
         // We want an exact match here to ensure that bucket names starting with
         // the same prefix are not mixed up.
@@ -103,7 +103,7 @@ public bool IsValidRequest(HttpContext context)
             if (nameToMatch.Equals(k, StringComparison.OrdinalIgnoreCase))
             {
                 bucketName = k;
-                s3Client = this.buckets[k];
+                s3Client = this.buckets[k].Client;
                 break;
             }
         }
@@ -190,14 +190,13 @@ private static async Task<KeyExistsResult> KeyExists(IAmazonS3 s3Client, string
     /// Releases the unmanaged resources used by the <see cref="AWSS3StorageImageProvider"/> and optionally releases the managed resources.
     /// </summary>
     /// <param name="disposing">true to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
-
     protected virtual void Dispose(bool disposing)
     {
         if (!this.isDisposed)
         {
             if (disposing)
             {
-                foreach (AmazonS3Client client in this.buckets.Values)
+                foreach (AmazonS3BucketClient client in this.buckets.Values)
                 {
                     client?.Dispose();
                 }

src/ImageSharp.Web.Providers.AWS/Providers/AWSS3StorageImageProviderOptions.cs

@@ -1,8 +1,6 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
-using Amazon.S3;
-
 namespace SixLabors.ImageSharp.Web.Providers.AWS;
 
 /// <summary>
@@ -22,7 +20,7 @@ public class AWSS3StorageImageProviderOptions
 public class AWSS3BucketClientOptions : IAWSS3BucketClientOptions
 {
     /// <inheritdoc/>
-    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3Client>? S3ClientFactory { get; set; }
+    public Func<IAWSS3BucketClientOptions, IServiceProvider, AmazonS3BucketClient>? S3ClientFactory { get; set; }
 
     /// <inheritdoc/>
     public string? Region { get; set; }

tests/ImageSharp.Web.Tests/TestUtilities/AWSS3StorageImageProviderFactory.cs

@@ -27,8 +27,8 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A
     {
         // Upload an image to the AWS Test Storage;
         AWSS3BucketClientOptions bucketOptions = options.S3Buckets.First();
-        AmazonS3Client amazonS3Client = AmazonS3ClientFactory.CreateClient(bucketOptions);
-        ListBucketsResponse listBucketsResponse = await amazonS3Client.ListBucketsAsync();
+        using AmazonS3BucketClient amazonS3Client = AmazonS3ClientFactory.CreateClient(bucketOptions);
+        ListBucketsResponse listBucketsResponse = await amazonS3Client.Client.ListBucketsAsync();
 
         bool foundBucket = false;
         foreach (S3Bucket b in listBucketsResponse.Buckets)
@@ -51,7 +51,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A
                     CannedACL = S3CannedACL.PublicRead
                 };
 
-                await amazonS3Client.PutBucketAsync(putBucketRequest);
+                await amazonS3Client.Client.PutBucketAsync(putBucketRequest);
             }
             catch (AmazonS3Exception e)
             {
@@ -76,7 +76,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A
                 Key = TestConstants.ImagePath
             };
 
-            await amazonS3Client.GetObjectAsync(request);
+            await amazonS3Client.Client.GetObjectAsync(request);
         }
         catch
         {
@@ -105,7 +105,7 @@ private static async Task InitializeAWSStorageAsync(IServiceProvider services, A
                 UseChunkEncoding = false,
             };
 
-            await amazonS3Client.PutObjectAsync(putRequest);
+            await amazonS3Client.Client.PutObjectAsync(putRequest);
         }
     }
 }