fix(infra): add JVM shutdown hooks to stop Testcontainers after test execution#189
Merged
Merged
Conversation
…execution Testcontainers started in static initializers were never stopped, causing orphaned PostgreSQL, Kafka, Redis, and Mailpit containers to accumulate after each test run (~30-50 containers, 5-15 GB leaked memory). Add Runtime.getRuntime().addShutdownHook() to all 10 AbstractIntegrationTest classes to ensure containers are stopped when the JVM exits. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughIntegration test modules across 10 microservices now include JVM shutdown hooks in their static initializers to gracefully stop Testcontainers (Redis, Kafka, PostgreSQL, Mailpit) on JVM termination, ensuring deterministic resource cleanup without altering startup sequences or public APIs. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@blockchain-custody/blockchain-custody/src/integration-test/java/com/stablecoin/payments/custody/AbstractIntegrationTest.java`:
- Around line 30-37: The static initializer currently starts POSTGRES then KAFKA
and registers a shutdown hook that calls KAFKA.stop() and POSTGRES.stop(), but
if KAFKA.stop() throws the POSTGRES.stop() call is skipped and startup
exceptions can leave containers running; change the startup sequence to start
each service with try/catch so if KAFKA.start() fails you stop POSTGRES
immediately, and only register the Runtime.getRuntime().addShutdownHook(...)
after both starts succeed; in the shutdown hook wrap each stop call in its own
try/catch (call KAFKA.stop() and POSTGRES.stop() independently) so one failure
does not prevent the other from running.
In
`@merchant-iam/merchant-iam/src/integration-test/java/com/stablecoin/payments/merchant/iam/AbstractIntegrationTest.java`:
- Around line 53-58: The shutdown hook currently calls REDIS.stop(),
MAILPIT.stop(), KAFKA.stop(), POSTGRES.stop() sequentially which can abort
remaining stops if one throws; add a private helper method stopQuietly(Startable
container) that wraps container.stop() in a try/catch and ignores exceptions,
then replace each direct call in the Runtime.getRuntime().addShutdownHook lambda
with stopQuietly(REDIS), stopQuietly(MAILPIT), stopQuietly(KAFKA),
stopQuietly(POSTGRES) so each container is best-effort stopped even if others
fail.
In
`@payment-orchestrator/payment-orchestrator/src/integration-test/java/com/stablecoin/payments/orchestrator/AbstractIntegrationTest.java`:
- Around line 31-37: The static initializer should be made failure-safe:
register the shutdown hook first (or ensure it tolerates partially-initialized
state) and start containers with per-step error handling so any
previously-started container is stopped if a later start fails; in the shutdown
hook call KAFKA.stop() and POSTGRES.stop() inside separate try/catch blocks (or
use try/finally) so an exception from one does not prevent the other from
stopping, and when starting in the static block ensure that on any exception you
stop already-started resources (KAFKA or POSTGRES) before rethrowing so no
containers are orphaned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 82484a0e-d53e-4caf-ad3d-4dd32d8b8e49
📒 Files selected for processing (10)
api-gateway-iam/api-gateway-iam/src/integration-test/java/com/stablecoin/payments/gateway/iam/AbstractIntegrationTest.javablockchain-custody/blockchain-custody/src/integration-test/java/com/stablecoin/payments/custody/AbstractIntegrationTest.javacompliance-travel-rule/compliance-travel-rule/src/integration-test/java/com/stablecoin/payments/compliance/AbstractIntegrationTest.javafiat-off-ramp/fiat-off-ramp/src/integration-test/java/com/stablecoin/payments/offramp/AbstractIntegrationTest.javafiat-on-ramp/fiat-on-ramp/src/integration-test/java/com/stablecoin/payments/onramp/AbstractIntegrationTest.javafx-liquidity-engine/fx-liquidity-engine/src/integration-test/java/com/stablecoin/payments/fx/AbstractIntegrationTest.javaledger-accounting/ledger-accounting/src/integration-test/java/com/stablecoin/payments/ledger/AbstractIntegrationTest.javamerchant-iam/merchant-iam/src/integration-test/java/com/stablecoin/payments/merchant/iam/AbstractIntegrationTest.javamerchant-onboarding/merchant-onboarding/src/integration-test/java/com/stablecoin/payments/merchant/onboarding/AbstractIntegrationTest.javapayment-orchestrator/payment-orchestrator/src/integration-test/java/com/stablecoin/payments/orchestrator/AbstractIntegrationTest.java
Comment on lines
30
to
37
| static { | ||
| POSTGRES.start(); | ||
| KAFKA.start(); | ||
| Runtime.getRuntime().addShutdownHook(new Thread(() -> { | ||
| KAFKA.stop(); | ||
| POSTGRES.stop(); | ||
| })); | ||
| } |
There was a problem hiding this comment.
Harden shutdown hook to avoid partial cleanup and startup leaks.
If KAFKA.stop() throws at Line 34, POSTGRES.stop() at Line 35 will not run. Also, a startup exception before hook registration can still leave orphaned containers.
Proposed hardening patch
static {
- POSTGRES.start();
- KAFKA.start();
- Runtime.getRuntime().addShutdownHook(new Thread(() -> {
- KAFKA.stop();
- POSTGRES.stop();
- }));
+ try {
+ POSTGRES.start();
+ KAFKA.start();
+ } catch (RuntimeException ex) {
+ safeStop(KAFKA);
+ safeStop(POSTGRES);
+ throw ex;
+ }
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+ safeStop(KAFKA);
+ safeStop(POSTGRES);
+ }, "testcontainers-shutdown"));
}
+
+private static void safeStop(org.testcontainers.containers.GenericContainer<?> container) {
+ try {
+ if (container != null && container.isRunning()) {
+ container.stop();
+ }
+ } catch (Exception ignored) {
+ // best-effort cleanup for test infra
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| static { | |
| POSTGRES.start(); | |
| KAFKA.start(); | |
| Runtime.getRuntime().addShutdownHook(new Thread(() -> { | |
| KAFKA.stop(); | |
| POSTGRES.stop(); | |
| })); | |
| } | |
| static { | |
| try { | |
| POSTGRES.start(); | |
| KAFKA.start(); | |
| } catch (RuntimeException ex) { | |
| safeStop(KAFKA); | |
| safeStop(POSTGRES); | |
| throw ex; | |
| } | |
| Runtime.getRuntime().addShutdownHook(new Thread(() -> { | |
| safeStop(KAFKA); | |
| safeStop(POSTGRES); | |
| }, "testcontainers-shutdown")); | |
| } | |
| private static void safeStop(org.testcontainers.containers.GenericContainer<?> container) { | |
| try { | |
| if (container != null && container.isRunning()) { | |
| container.stop(); | |
| } | |
| } catch (Exception ignored) { | |
| // best-effort cleanup for test infra | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@blockchain-custody/blockchain-custody/src/integration-test/java/com/stablecoin/payments/custody/AbstractIntegrationTest.java`
around lines 30 - 37, The static initializer currently starts POSTGRES then
KAFKA and registers a shutdown hook that calls KAFKA.stop() and POSTGRES.stop(),
but if KAFKA.stop() throws the POSTGRES.stop() call is skipped and startup
exceptions can leave containers running; change the startup sequence to start
each service with try/catch so if KAFKA.start() fails you stop POSTGRES
immediately, and only register the Runtime.getRuntime().addShutdownHook(...)
after both starts succeed; in the shutdown hook wrap each stop call in its own
try/catch (call KAFKA.stop() and POSTGRES.stop() independently) so one failure
does not prevent the other from running.
Comment on lines
+53
to
+58
| Runtime.getRuntime().addShutdownHook(new Thread(() -> { | ||
| REDIS.stop(); | ||
| MAILPIT.stop(); | ||
| KAFKA.stop(); | ||
| POSTGRES.stop(); | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify shutdown hooks with unguarded sequential stop() calls.
# Expected after fix: no matches.
fd 'AbstractIntegrationTest.java' \
| xargs rg -nU 'addShutdownHook\(new Thread\(\(\) -> \{\n(?:\s+[A-Z_]+\.stop\(\);\n)+\s+\}\)\);'Repository: Puneethkumarck/stablebridge-platform
Length of output: 6951
🏁 Script executed:
cat -n merchant-iam/merchant-iam/src/integration-test/java/com/stablecoin/payments/merchant/iam/AbstractIntegrationTest.java | sed -n '50,65p'Repository: Puneethkumarck/stablebridge-platform
Length of output: 553
Wrap container shutdowns to prevent cleanup chain-breaks.
Sequential unguarded stop() calls on lines 54–57 will skip remaining containers if any throws. Introduce a helper to isolate failures and ensure best-effort cleanup:
private static void stopQuietly(Startable container) {
try {
container.stop();
} catch (Exception ignored) {
// best-effort during JVM shutdown
}
}Then replace each CONTAINER.stop(); with stopQuietly(CONTAINER);.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@merchant-iam/merchant-iam/src/integration-test/java/com/stablecoin/payments/merchant/iam/AbstractIntegrationTest.java`
around lines 53 - 58, The shutdown hook currently calls REDIS.stop(),
MAILPIT.stop(), KAFKA.stop(), POSTGRES.stop() sequentially which can abort
remaining stops if one throws; add a private helper method stopQuietly(Startable
container) that wraps container.stop() in a try/catch and ignores exceptions,
then replace each direct call in the Runtime.getRuntime().addShutdownHook lambda
with stopQuietly(REDIS), stopQuietly(MAILPIT), stopQuietly(KAFKA),
stopQuietly(POSTGRES) so each container is best-effort stopped even if others
fail.
Comment on lines
31
to
+37
| static { | ||
| POSTGRES.start(); | ||
| KAFKA.start(); | ||
| Runtime.getRuntime().addShutdownHook(new Thread(() -> { | ||
| KAFKA.stop(); | ||
| POSTGRES.stop(); | ||
| })); |
There was a problem hiding this comment.
Make container cleanup failure-safe in startup and shutdown paths.
If KAFKA.stop() fails at Line 35, POSTGRES.stop() at Line 36 is skipped. Also, startup failures before hook registration can still orphan already-started containers.
Proposed hardening patch
static {
- POSTGRES.start();
- KAFKA.start();
- Runtime.getRuntime().addShutdownHook(new Thread(() -> {
- KAFKA.stop();
- POSTGRES.stop();
- }));
+ try {
+ POSTGRES.start();
+ KAFKA.start();
+ } catch (RuntimeException ex) {
+ safeStop(KAFKA);
+ safeStop(POSTGRES);
+ throw ex;
+ }
+ Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+ safeStop(KAFKA);
+ safeStop(POSTGRES);
+ }, "testcontainers-shutdown"));
}
+
+private static void safeStop(org.testcontainers.containers.GenericContainer<?> container) {
+ try {
+ if (container != null && container.isRunning()) {
+ container.stop();
+ }
+ } catch (Exception ignored) {
+ // best-effort cleanup for test infra
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@payment-orchestrator/payment-orchestrator/src/integration-test/java/com/stablecoin/payments/orchestrator/AbstractIntegrationTest.java`
around lines 31 - 37, The static initializer should be made failure-safe:
register the shutdown hook first (or ensure it tolerates partially-initialized
state) and start containers with per-step error handling so any
previously-started container is stopped if a later start fails; in the shutdown
hook call KAFKA.stop() and POSTGRES.stop() inside separate try/catch blocks (or
use try/finally) so an exception from one does not prevent the other from
stopping, and when starting in the static block ensure that on any exception you
stop already-started resources (KAFKA or POSTGRES) before rethrowing so no
containers are orphaned.
3 tasks
4 tasks
5 tasks
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
Runtime.getRuntime().addShutdownHook()to all 10AbstractIntegrationTestclassesTest plan
make test-integrationand verifydocker psshows 0 Testcontainers after completion🤖 Generated with Claude Code
Summary by CodeRabbit