[konlue]

Fixes #7774.

Summary

ShenyuClusterService owns the election scheduler but its shutdown() — wired as the bean destroy method in ClusterConfiguration — is empty, so election/renewal activity keeps running against dependencies being destroyed after the shutdown contract returns.

Modifications

• Implement shutdown(): shutdownNow() on the owned scheduler followed by a bounded awaitTermination (5s; warn and continue on timeout so bean destruction never hangs). Idempotent, and a no-op when the task never started.
• The interrupt surfaces inside the renewal loop, which releases leadership through its existing finally releaseMaster() and closes the upstream/instance check services via its existing error path — no locks are unlocked from another thread.

Verifying this change

• New ShenyuClusterServiceTest (first test for the cluster module): after shutdown(), isShutdown() is true and no further election attempts occur; shutdown before start does not throw; shutdown during an active renewal loop releases leadership (releaseMaster) and closes both check services. All fail on the previous empty shutdown() and pass with the fix.

• Targeted tests pass:

  ./mvnw -B -ntp -pl shenyu-admin test -Dtest=ShenyuClusterServiceTest

  3/3 green.

• Checkstyle passes:

  ./mvnw -B -ntp -pl shenyu-admin checkstyle:check

  clean.

Notes

If the renewal thread is inside a non-interruptible call when shutdown arrives, the bounded wait logs and returns rather than blocking context destruction. The interrupt is recorded by the existing select master error log line — expected shutdown noise. Leadership is never released from a foreign thread.

[Aias00]

Approving. The shutdown contract was genuinely empty and the scheduler is owned by this bean, so implementing the destroy method is the right fix; I verified that the teardown path it triggers is safe to run twice.

Verification

• executorService is created in the constructor (:54-56), so shutdown() before start() cannot NPE — the new test covering that case is testing a real ordering, because ApplicationStartListener and bean destruction are not guaranteed to be paired.
• The interrupt surfaces in TimeUnit.SECONDS.sleep:93 inside the renewal loop, which means finally releaseMaster():114 runs and the existing catch (Exception):104-108 closes upstreamCheckService / instanceCheckService. I checked that the close path is idempotent before accepting it as safe on a normal shutdown: UpstreamCheckService#close:177-189 cancels the future and shuts the two executors down behind null guards, so the later @PreDestroy invocation from Spring is a no-op rather than a second teardown of a live pool.
• Nothing unlocks a lock from a foreign thread: shutdownNow() only interrupts, and the lock release still happens on the scheduler thread inside the finally. ScheduledThreadPoolExecutor swallows the ShenyuException rethrown at :111 into the task's future, so it cannot leak into the shutdown caller.
• shutdown() before start() is also the realistic ordering for the bean itself: ClusterConfiguration creates it eagerly, while the task is only scheduled on start().

One thing I would change (not blocking)

shutdownNow() interrupting the renewal loop means the existing catch (Exception e) at :104 logs select master error at ERROR with an InterruptedException stack trace on every graceful shutdown of a cluster-mode admin. That is exactly the kind of line SREs page on, and it will now fire on every planned restart. Two cheap options:

• a volatile boolean shuttingDown set at the top of shutdown(), and inside doSelectMaster log at INFO (or skip the error log) when it is set, while still running releaseMaster() and closing the check services;
• or check executorService.isShutdown() in the catch block.

The author's note acknowledges the noise, so this is a preference about production logs rather than a defect — but since the whole point of the PR is a clean shutdown, an ERROR-per-shutdown slightly undercuts it.

Related, and worth one sentence in the javadoc: if the renewal thread is inside a non-interruptible selectMaster/checkMasterStatus (JDBC, ZK), awaitTermination gives up after 5s and bean destruction proceeds while that thread may still be writing. Leadership is still released when the call returns, but a reader should not assume "shutdown returned" implies "scheduler stopped". The bounded wait plus the warning log is the right trade-off; it just needs to be stated.

Also noted: the catch does not restore the interrupt flag. Harmless here (the thread is terminating, and shutdown() re-interrupts the caller on InterruptedException), but if the loop ever gains a restart path the signal would be lost.

Test

testShutdownInterruptsRenewalAndReleasesLeadership is the case worth having, and with selectPeriod=1 the interrupt reliably lands in sleep, so it is not timing-fragile. The 2.5s settle in testShutdownStopsElectionScheduler is above the 1s period, so the "no attempt after shutdown" assertion is meaningful rather than vacuous.

CI on 6fb77c3 is green (pr_build, it, it-k8s, e2e).