RockteMQ-AI commented on issue #11161:
URL: https://github.com/apache/rocketmq/issues/11161#issuecomment-5660904949
**Issue Evaluation**
Category: `bug` | Status: **Confirmed**
Both defects described in this issue have been verified against the current
`develop` branch source code.
### Bug 1 — Inverted condition in `closeChannel(String, Channel)`
**Confirmed.** At line 439 of `NettyRemotingClient.java`:
```java
} else if (prevCW.isWrapperOf(channel)) {
LOGGER.info("... has been closed before, and has been created again,
nothing to do.", ...);
removeItemFromTable = false;
}
```
`isWrapperOf(channel)` returns `true` when the wrapper stored under `addr`
wraps *this* channel (the normal close path). The branch should fire when the
wrapper does **not** wrap this channel (i.e., a new channel has already
replaced it), so the condition needs to be negated:
`!prevCW.isWrapperOf(channel)`.
This was introduced by #8366 which replaced `prevCW.getChannel() != channel`
with `isWrapperOf(channel)` without re-negating. The practical effect is that
the eager eviction path is dead for the common case, and cleanup relies on the
fragile pipeline-event detour through `closeChannel(Channel)`.
### Bug 2 — AB-BA lock ordering in `ChannelWrapper.close()`
**Confirmed.** The lock acquisition order is inverted:
| Method | Lock order |
|--------|-----------|
| `closeChannel(String, Channel)` | `lockChannelTables` → wrapper `readLock`
(via `tryClose`) |
| `ChannelWrapper.close()` | wrapper `writeLock` → `lockChannelTables` (via
`closeChannel`) |
This is a classic AB-BA lock ordering inversion that can lead to deadlock
under concurrent close operations.
### Severity
- **Bug 1**: Medium — functional correctness issue; channel table entries
are not eagerly evicted, leading to stale entries and misleading log messages.
- **Bug 2**: High — potential deadlock under concurrent close operations.
### Suggested Fix
1. Negate the condition: `else if (!prevCW.isWrapperOf(channel))`
2. In `ChannelWrapper.close()`, acquire `lockChannelTables` before the
wrapper write lock, consistent with the rest of the class.
An automated fix proposal can be generated. Reply `/approve` to proceed with
PR generation.
---
*Automated evaluation by RockteMQ-AI*
--
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]
