apache / apache/shenyu

[BUG] ConsulInstanceRegisterRepository.close() never shuts down executor or TtlScheduler — non-daemon thread blocks JVM shutdown

Closed
#6,858 2 comments 0 reactions 1 assignee Claimed by @yykaue View on GitHub
client: register priority: medium type: bug
Dominant language
Java
Stars
8.8k
Forks
3.1k
Avg merge
7d 1h
Merged PRs (30d)
85

Description

## Description
`close()` (lines 141-150) cancels the `watchFutures` and deregisters the service, but never calls `executor.shutdown()` or `ttlScheduler` shutdown. The `executor` field (line 69) is a `ScheduledThreadPoolExecutor` with daemon threads (minor leak on registry re-creation). More critically, `TtlScheduler` (line 41) creates its scheduler via `Executors.newSingleThreadScheduledExecutor()`, which uses the default thread factory producing a **non-daemon** thread. This thread is never shut down — `close()` only calls `ttlScheduler.remove(newService.getId())` which cancels the heartbeat task but leaves the scheduler alive. Since `@Join(isSingleton = false)` creates a new repository instance per discovery, each `AbstractDiscoveryProcessor.removeDiscovery` → `close()` cycle leaks a non-daemon scheduler thread. On graceful Spring context close, the non-daemon TtlScheduler thread prevents JVM exit: the JVM will not start its shutdown sequence while a non-daemon thread is alive, and the shutdown hook that calls `close()` only fires after shutdown begins — a deadlock.

## Location
- `shenyu-registry/shenyu-registry-consul/src/main/java/org/apache/shenyu/registry/consul/ConsulInstanceRegisterRepository.java:69,141-150`
- `shenyu-registry/shenyu-registry-consul/src/main/java/org/apache/shenyu/registry/consul/TtlScheduler.java:41`

## Impact
JVM hang on graceful shutdown (the non-daemon TtlScheduler thread blocks exit). Thread leak on each discovery add/remove cycle in the admin, eventually exhausting thread limits.

## Suggested fix
Add a `shutdown()` method to `TtlScheduler` that calls `scheduler.shutdownNow()`, and call it from `ConsulInstanceRegisterRepository.close()`. Also call `executor.shutdownNow()` in `close()`. Alternatively, make the TtlScheduler's thread factory daemon: `Executors.newSingleThreadScheduledExecutor(ShenyuThreadFactory.create("consul-ttl", true))`.

## Related existing
None — distinct from N34 (#6703, `UpstreamCheckService.close` leaks invokeExecutor) which covers the admin-side `shenyu-admin/.../UpstreamCheckService`, not the `shenyu-registry-consul` module. The shutdown-hook deadlock via non-daemon thread is a distinct root cause.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.