From 211a5a853aafb490d9f82d6f6b542d58d2b5b5f8 Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 26 Jul 2026 12:21:38 +0900 Subject: [PATCH 1/2] Dispose the ACME client and signer when context creation fails CreateClientCoreAsync built an AcmeClient, which owns an HttpClient, before fetching the directory and creating the account. The context is only assigned to the cached field on success, so any failure in between left the client and its signer unreachable and undisposed. Against a persistently failing ACME endpoint this accumulated one HttpClient per retry for the life of the worker. Co-Authored-By: Claude Opus 5 --- src/Acmebot.App/Acme/AcmeClientFactory.cs | 85 ++++++++++++++--------- 1 file changed, 51 insertions(+), 34 deletions(-) diff --git a/src/Acmebot.App/Acme/AcmeClientFactory.cs b/src/Acmebot.App/Acme/AcmeClientFactory.cs index 61199606..94f9eb66 100644 --- a/src/Acmebot.App/Acme/AcmeClientFactory.cs +++ b/src/Acmebot.App/Acme/AcmeClientFactory.cs @@ -63,53 +63,70 @@ private async Task CreateClientCoreAsync() } var signer = accountKey.GenerateSigner(); - var client = new AcmeClient( - _options.Endpoint, - new AcmeClientOptions - { - UserAgent = $"Acmebot/{Constants.ApplicationVersion}" - }); - var directory = await client.GetDirectoryAsync(); - AcmeAccountHandle accountHandle; + AcmeClient? client = null; - if (account is null) + // The client owns an HttpClient, and both it and the signer are only handed to the caller once + // the context has been fully built. Anything that fails in between (a directory fetch against an + // unreachable ACME endpoint, missing EAB credentials, a state store write) would otherwise leak + // them on every retry. + try { - var externalAccountBinding = CreateExternalAccountBinding(); + client = new AcmeClient( + _options.Endpoint, + new AcmeClientOptions + { + UserAgent = $"Acmebot/{Constants.ApplicationVersion}" + }); - if (externalAccountBinding is null && (directory.Metadata?.ExternalAccountRequired ?? false)) + var directory = await client.GetDirectoryAsync(); + AcmeAccountHandle accountHandle; + + if (account is null) { - throw new PreconditionException("This ACME endpoint requires External Account Binding (EAB). Configure EAB credentials and try again."); - } + var externalAccountBinding = CreateExternalAccountBinding(); - accountHandle = await client.CreateAccountAsync( - signer, - new AcmeNewAccountRequest + if (externalAccountBinding is null && (directory.Metadata?.ExternalAccountRequired ?? false)) + { + throw new PreconditionException("This ACME endpoint requires External Account Binding (EAB). Configure EAB credentials and try again."); + } + + accountHandle = await client.CreateAccountAsync( + signer, + new AcmeNewAccountRequest + { + Contact = contacts, + TermsOfServiceAgreed = true + }, + externalAccountBinding); + account = AccountDetails.FromAccountHandle(accountHandle, directory.Metadata?.TermsOfService); + + if (isNewAccountKey) { - Contact = contacts, - TermsOfServiceAgreed = true - }, - externalAccountBinding); - account = AccountDetails.FromAccountHandle(accountHandle, directory.Metadata?.TermsOfService); + await stateStore.SaveAsync(accountKey, "account_key.json"); + } - if (isNewAccountKey) + await stateStore.SaveAsync(account, "account.json"); + } + else { - await stateStore.SaveAsync(accountKey, "account_key.json"); + accountHandle = account.ToAccountHandle(signer); } - await stateStore.SaveAsync(account, "account.json"); + return new AcmeClientContext + { + Client = client, + Directory = directory, + Signer = signer, + Account = accountHandle + }; } - else + catch { - accountHandle = account.ToAccountHandle(signer); - } + client?.Dispose(); + signer.Dispose(); - return new AcmeClientContext - { - Client = client, - Directory = directory, - Signer = signer, - Account = accountHandle - }; + throw; + } } private AcmeExternalAccountBindingOptions? CreateExternalAccountBinding() From cfd0ce504cc48dfb7e662c310a1810a8167c2d96 Mon Sep 17 00:00:00 2001 From: Tatsuro Shibamura Date: Sun, 26 Jul 2026 12:35:35 +0900 Subject: [PATCH 2/2] Match AcmeClientContext dispose order on the failure path Co-Authored-By: Claude Opus 5 --- src/Acmebot.App/Acme/AcmeClientFactory.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Acmebot.App/Acme/AcmeClientFactory.cs b/src/Acmebot.App/Acme/AcmeClientFactory.cs index 94f9eb66..57c7aca9 100644 --- a/src/Acmebot.App/Acme/AcmeClientFactory.cs +++ b/src/Acmebot.App/Acme/AcmeClientFactory.cs @@ -122,8 +122,9 @@ private async Task CreateClientCoreAsync() } catch { - client?.Dispose(); + // Same order as AcmeClientContext.Dispose(), which owns these once the context exists. signer.Dispose(); + client?.Dispose(); throw; }