zjncs opened a new pull request, #11119:
URL: https://github.com/apache/rocketmq/pull/11119
## Motivation
Eleven `RouteInfoManager` methods acquire the route lock with
`lockInterruptibly()` and release it in a `finally` that sits **outside** the
catch clause:
```java
try {
this.lock.writeLock().lockInterruptibly();
...
} catch (Exception e) {
log.error("registerTopic Exception", e);
} finally {
this.lock.writeLock().unlock(); // runs even if lockInterruptibly()
threw
}
```
When the thread is interrupted while waiting for the lock (e.g. namesrv
shutdown interrupts request processor threads that are contending for the route
lock), `lockInterruptibly()` throws `InterruptedException`, the catch clause
logs it, and the `finally` block then calls `unlock()` on a lock the current
thread never acquired — `IllegalMonitorStateException` escapes the method to
the request processor, turning a graceful degradation into a spurious error.
Affected methods: `registerTopic`, both `deleteTopic` overloads,
`registerBroker`, `unregisterBroker`, `pickupTopicRouteData`,
`getAllTopicList`, `getSystemTopicList`, `getUnitTopics`,
`getHasUnitSubTopicList`, `getHasUnitSubUnUnitTopicList`.
## Modification
Guard each of the eleven `unlock()` calls so the lock is only released when
the current thread actually holds it:
- write locks: `if (this.lock.isWriteLockedByCurrentThread())`
- read locks: `if (this.lock.getReadHoldCount() > 0)`
The `lock` field declaration changes from the `ReadWriteLock` interface to
`ReentrantReadWriteLock` (the implementation it always was) so the guard
methods are visible. No body is re-indented; behavior of every method is
unchanged except that an interrupt during lock acquisition no longer leaks
`IllegalMonitorStateException`.
## Test Evidence
**Fail-before** (unpatched code, new tests
`RouteInfoManagerTest#testRegisterTopicWithInterruptedThread` /
`#testGetAllTopicListWithInterruptedThread` — interrupt the current thread,
then call the method):
```
docker exec rmq-build mvn -q -pl namesrv test
-Dtest='RouteInfoManagerTest#testRegisterTopicWithInterruptedThread+testGetAllTopicListWithInterruptedThread'
-Dsurefire.failIfNoSpecifiedTests=true
Tests run: 2, Errors: 1 ... java.lang.IllegalMonitorStateException
```
**Pass-after** (full class with the fix):
```
docker exec rmq-build mvn -q -pl namesrv test -Dtest='RouteInfoManagerTest'
-Dsurefire.failIfNoSpecifiedTests=true
Tests run: 14, Failures: 0, Errors: 0, Skipped: 0
```
Note: `RequestProcessorTest#testBrokerHeartBeat` errors with
`NoSuchFieldException: modifiers` on this JDK both before and after the change
(pre-existing, JDK reflection restriction), verified by running it on pristine
develop.
No associated issue (self-discovered during a namesrv-module self-audit).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
