Reuse SesClient in AwsMailProvider#1044
Open
pditommaso wants to merge 2 commits into
Open
Conversation
`AwsMailProvider.send()` was building a fresh `SesClient` per invocation and never closing it, leaving the underlying Apache HTTP client, connection pool and SSL context alive. `AwsEcrService.stsClient(region)` likewise built a new `DefaultCredentialsProvider` on every call; closing the STS client does not close the provider, so its IMDS/container credential refresh HTTP clients and background executors leaked. Both clients are now created once per process (via `@PostConstruct`) and released on shutdown (`@PreDestroy`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…plied credentials provider
Empirically verified with a tracking wrapper: StsClient.close() invokes
close() twice on the user-supplied AwsCredentialsProvider via the
internal AttributeMap.close() path. Sharing a single
DefaultCredentialsProvider across STS clients wrapped in
withCloseable { ... } would therefore tear down its underlying
ContainerCredentialsProvider / InstanceProfileCredentialsProvider
(HTTP client + refresh executor) after the first call, breaking
subsequent credential refreshes in prod where EKS pod identity or EC2
IMDS resolves.
Restoring the per-call DefaultCredentialsProvider.builder().build()
ephemeral instance — wasteful but correct. The leak rate (~1 STS call
per day in prod per investigation) does not justify a more invasive
restructuring. The AwsMailProvider singleton fix is kept since that
path has no withCloseable around the client.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pditommaso
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AwsMailProvider.send()builds a freshSesClientper invocation and never closes it — every send leaks the underlying Apache HTTP client, connection pool and SSL context. Hoisted to a singleton instance initialized in@PostConstructand closed in@PreDestroy.AwsEcrService.stsClient(region)builds a newDefaultCredentialsProvideron every call. Closing the STS client does not close the provider, so its IMDS/container credential refresh HTTP clients and background executors stay alive. Replaced with a single provider initialized in@PostConstructand closed in@PreDestroy;stsClient(String region)is now non-static so it can use the shared field.The other
stsClient(String region, Credentials)overload uses aStaticCredentialsProvider, which is a stateless POJO — left as-is.Background
While investigating a suspected memory leak in prod (which turned out to be JVM warmup transient — slope reverses after pods stabilize), the code review surfaced these two real leaks. Volume is low (
Mail message sent~110/day across pods,Assuming AWS role~1/day), so neither materially contributes to RSS in current prod traffic — but both are strict resource leaks that compound over long-lived pods.Test plan
./gradlew :compileGroovy— passes./gradlew :test --tests '*AwsEcrServiceTest*'— 48/48 pass, 0 failuresDefaultCredentialsProvideracross STS clients)🤖 Generated with Claude Code