Skip to content

JmsHealthIndicator leaks watchdog threads if start fails immediately - #51412

Merged
mhalbritter merged 1 commit into
spring-projects:4.0.xfrom
2heunxun:fix/jms-health-indicator-watchdog-thread-leak-4.0.x
Aug 25, 2026
Merged

JmsHealthIndicator leaks watchdog threads if start fails immediately#51412
mhalbritter merged 1 commit into
spring-projects:4.0.xfrom
2heunxun:fix/jms-health-indicator-watchdog-thread-leak-4.0.x

Conversation

@2heunxun

Copy link
Copy Markdown
Contributor

What happened?

JmsHealthIndicator.MonitoredConnection.start() spawns a watchdog
thread that waits on a CountDownLatch for up to 5 seconds to detect
a hanging connection.start() call. The latch is only counted down
after connection.start() returns:

this.connection.start();
this.latch.countDown();

If connection.start() throws a JMSException immediately (a common case, e.g. when the broker rejects authentication or refuses the connection outright), the countDown() call is skipped.

The watchdog thread then blocks for the full 5 seconds for no reason. After that, it logs a misleading "Connection failed to start within 5 seconds and will be closed" warning—the connection did not time out, it failed immediately—and calls close() on a connection that has already been closed by the enclosing try-with-resources in doHealthCheck.

Under frequent health polling against a broker that fails start() immediately (for example, a Kubernetes liveness probe hitting /actuator/health every 1–2 seconds during an outage), a new watchdog thread is spawned on every poll and each one lingers for the full 5 seconds. As a result, threads accumulate for the duration of the outage.

The watchdog thread was also not a daemon thread, so in extreme cases it could delay JVM shutdown.

What should happen?

The watchdog should be released immediately whether connection.start() succeeds or throws, and it should not delay JVM shutdown.

How to reproduce?

Added JmsHealthIndicatorTests#whenConnectionStartThrowsWatchdogThreadDoesNotAlsoCloseConnection.

The test mocks Connection.start() to throw immediately, then asserts that Connection.close() is invoked exactly once (by the try-with-resources) after waiting past the 5-second watchdog window.

On the current code, this test fails with TooManyActualInvocations because the watchdog performs a second, redundant close() call once the timeout elapses.

What does this PR change?

MonitoredConnection.start() now wraps connection.start() in a try/finally block so latch.countDown() always runs.
The watchdog thread is also marked as a daemon thread.
No public API changes are introduced. Behavior for the success path and the genuine-hang path remains unchanged.

Impact / risk

  • Success path (broker reachable): Identical behavior and response time. No impact.
  • Genuine hang path: Unchanged. The connection is still closed after 5 seconds.
  • Immediate-failure path: The health status remains DOWN. The only behavioral change is that close() is called once instead of twice, and the misleading timeout warning is no longer logged for a failure that was not actually a timeout.
  • Thread usage: On the immediate-failure path, the watchdog thread is now released immediately instead of blocking for up to 5 seconds, reducing thread accumulation under repeated failures such as frequent liveness-probe polling during an outage.
  • JVM shutdown: The watchdog thread is now a daemon thread and therefore cannot prevent JVM shutdown.

close() is idempotent per the JMS specification, so the previous double-close was not unsafe, but it was unnecessary.

Tests

  • Added the regression test above (fails before this change, passes after).
  • Existing JmsHealthIndicatorTests (6 tests) continue to pass.
  • checkstyleMain, checkstyleTest, checkFormatMain, checkFormatTest, and :module:spring-boot-jms:check all pass.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 24, 2026
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 24, 2026
@wilkinsona wilkinsona added this to the 4.0.x milestone Aug 24, 2026
@mhalbritter mhalbritter self-assigned this Aug 25, 2026
@mhalbritter
mhalbritter force-pushed the fix/jms-health-indicator-watchdog-thread-leak-4.0.x branch from 782388c to d00566a Compare August 25, 2026 07:13
@mhalbritter mhalbritter changed the title Fix watchdog thread leak in JmsHealthIndicator on start failure JmsHealthIndicator leaks watchdog threads if start fails immediately Aug 25, 2026
@mhalbritter
mhalbritter merged commit a39ed77 into spring-projects:4.0.x Aug 25, 2026
1 check passed
@mhalbritter

Copy link
Copy Markdown
Contributor

Thank you very much and congratulations on your first contribution 🎉!

@mhalbritter mhalbritter modified the milestones: 4.0.x, 4.0.9 Aug 25, 2026
@2heunxun

Copy link
Copy Markdown
Contributor Author

@mhalbritter Thank you so much! I’m really happy to have made my first contribution. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants