[iloveeyjafjalla]

Which Issue(s) This PR Fixes

Fixes #11456

Problem

RemotingProtocolServer's constructor schedules cleanExpireRequest on a single-thread ScheduledExecutorService built with ThreadFactoryBuilder (non-daemon threads):

this.timerExecutor.scheduleAtFixedRate(this::cleanExpireRequest, 100, 100, TimeUnit.MILLISECONDS);

shutdown() stops the eight remoting executors and leaves timerExecutor running, so a shut-down server keeps a live thread running the 100 ms cleanup task against stopped state, and the non-daemon thread keeps the JVM alive after the context closes. This is the same shape as #11427 (a service created in a constructor that the owner's shutdown never stops).

Change

Stop the timer with the other executors:

        this.defaultExecutor.shutdown();
+       // The constructor schedules the expired-request cleanup on this executor; leaving it running
+       // keeps a non-daemon thread (and its 100ms task) alive after the server has shut down.
+       this.timerExecutor.shutdown();
     }

Verification

RemotingProtocolServerTest gains testShutdownStopsTheExpiredRequestTimer, which injects mocks for every collaborator shutdown() touches and verifies the timer is stopped. With the old code:

Wanted but not invoked:
scheduledExecutorService.shutdown();
Tests run: 2, Failures: 1, Errors: 0, Skipped: 1

After the change, with JDK 8 (mvn -B -ntp -pl proxy -am test -Dtest='RemotingProtocolServerTest' -DfailIfNoTests=false -Djacoco.skip=true):

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
spotbugs:4.8.6.8:check: BugInstance size is 0
checkstyle:3.2.0:check (validate): passed

The test pins the whole shutdown contract, so a future executor started in this class cannot be added without also being stopped. The change was prepared with AI assistance and reviewed, tested and verified by me.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.74%. Comparing base (57c2dc0) to head (4a45d9e).

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #11457      +/-   ##
=============================================
+ Coverage      49.72%   49.74%   +0.01%     
- Complexity     14366    14367       +1     
=============================================
  Files           1390     1390              
  Lines         103264   103265       +1     
  Branches       13504    13504              
=============================================
+ Hits           51351    51369      +18     
+ Misses         45707    45691      -16     
+ Partials        6206     6205       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fix resource leak: stop the expired-request timer executor during server shutdown.

Findings

• [Info] proxy/src/main/java/org/apache/rocketmq/proxy/remoting/RemotingProtocolServer.java:263 — The fix correctly addresses a resource leak where timerExecutor was not shut down, leaving a non-daemon thread running the 100ms cleanup task after server shutdown. This matches the pattern of #11427.

Suggestions

The change is minimal, correct, and well-tested. The new test testShutdownStopsTheExpiredRequestTimer properly verifies the shutdown contract by injecting mocks for all executors and verifying timerExecutor.shutdown() is called exactly once.

Analysis

• Correctness: ✅ The fix is correct — all executors started in the constructor should be stopped in shutdown()
• Resource management: ✅ Prevents non-daemon thread leak that would keep JVM alive after context close
• Test coverage: ✅ Excellent test that pins the entire shutdown contract
• Backward compatibility: ✅ No API changes

LGTM. Clean fix for a legitimate resource leak.

---

Automated review by github-manager-bot