Skip to content

Commit 71e77ef

Browse files
shibayanclaude
andauthored
Dispose the ACME client and signer when context creation fails (#1222)
* 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 <noreply@anthropic.com> * Match AcmeClientContext dispose order on the failure path Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent d720eb3 commit 71e77ef

1 file changed

Lines changed: 52 additions & 34 deletions

File tree

src/Acmebot.App/Acme/AcmeClientFactory.cs

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -63,53 +63,71 @@ private async Task<AcmeClientContext> CreateClientCoreAsync()
6363
}
6464

6565
var signer = accountKey.GenerateSigner();
66-
var client = new AcmeClient(
67-
_options.Endpoint,
68-
new AcmeClientOptions
69-
{
70-
UserAgent = $"Acmebot/{Constants.ApplicationVersion}"
71-
});
72-
var directory = await client.GetDirectoryAsync();
73-
AcmeAccountHandle accountHandle;
66+
AcmeClient? client = null;
7467

75-
if (account is null)
68+
// The client owns an HttpClient, and both it and the signer are only handed to the caller once
69+
// the context has been fully built. Anything that fails in between (a directory fetch against an
70+
// unreachable ACME endpoint, missing EAB credentials, a state store write) would otherwise leak
71+
// them on every retry.
72+
try
7673
{
77-
var externalAccountBinding = CreateExternalAccountBinding();
74+
client = new AcmeClient(
75+
_options.Endpoint,
76+
new AcmeClientOptions
77+
{
78+
UserAgent = $"Acmebot/{Constants.ApplicationVersion}"
79+
});
7880

79-
if (externalAccountBinding is null && (directory.Metadata?.ExternalAccountRequired ?? false))
81+
var directory = await client.GetDirectoryAsync();
82+
AcmeAccountHandle accountHandle;
83+
84+
if (account is null)
8085
{
81-
throw new PreconditionException("This ACME endpoint requires External Account Binding (EAB). Configure EAB credentials and try again.");
82-
}
86+
var externalAccountBinding = CreateExternalAccountBinding();
8387

84-
accountHandle = await client.CreateAccountAsync(
85-
signer,
86-
new AcmeNewAccountRequest
88+
if (externalAccountBinding is null && (directory.Metadata?.ExternalAccountRequired ?? false))
89+
{
90+
throw new PreconditionException("This ACME endpoint requires External Account Binding (EAB). Configure EAB credentials and try again.");
91+
}
92+
93+
accountHandle = await client.CreateAccountAsync(
94+
signer,
95+
new AcmeNewAccountRequest
96+
{
97+
Contact = contacts,
98+
TermsOfServiceAgreed = true
99+
},
100+
externalAccountBinding);
101+
account = AccountDetails.FromAccountHandle(accountHandle, directory.Metadata?.TermsOfService);
102+
103+
if (isNewAccountKey)
87104
{
88-
Contact = contacts,
89-
TermsOfServiceAgreed = true
90-
},
91-
externalAccountBinding);
92-
account = AccountDetails.FromAccountHandle(accountHandle, directory.Metadata?.TermsOfService);
105+
await stateStore.SaveAsync(accountKey, "account_key.json");
106+
}
93107

94-
if (isNewAccountKey)
108+
await stateStore.SaveAsync(account, "account.json");
109+
}
110+
else
95111
{
96-
await stateStore.SaveAsync(accountKey, "account_key.json");
112+
accountHandle = account.ToAccountHandle(signer);
97113
}
98114

99-
await stateStore.SaveAsync(account, "account.json");
115+
return new AcmeClientContext
116+
{
117+
Client = client,
118+
Directory = directory,
119+
Signer = signer,
120+
Account = accountHandle
121+
};
100122
}
101-
else
123+
catch
102124
{
103-
accountHandle = account.ToAccountHandle(signer);
104-
}
125+
// Same order as AcmeClientContext.Dispose(), which owns these once the context exists.
126+
signer.Dispose();
127+
client?.Dispose();
105128

106-
return new AcmeClientContext
107-
{
108-
Client = client,
109-
Directory = directory,
110-
Signer = signer,
111-
Account = accountHandle
112-
};
129+
throw;
130+
}
113131
}
114132

115133
private AcmeExternalAccountBindingOptions? CreateExternalAccountBinding()

0 commit comments

Comments
 (0)