[Bug] MetaLockUtils.commitLockTables leaks acquired commit locks on rollback (unlocks wrong index)
- Dominant language
- Java
- Stars
- 15.9k
- Forks
- 3.9k
- Avg merge
- 2d 23h
- Merged PRs (30d)
- 520
Description
### Search before asking
- [x] I had searched in the [issues](https://github.com/apache/doris/issues) and found no similar issues.
### Version
master (`MetaLockUtils.commitLockTables`).
### What's Wrong?
`MetaLockUtils.commitLockTables` has a rollback bug: when `commitLock()` throws for the table at index `i`, the rollback loop iterates `j` from `i-1` down to `0` but unlocks `tableList.get(i)` (the table that failed to lock) instead of `tableList.get(j)` (the previously locked tables):
```java
public static void commitLockTables(List tableList) {
for (int i = 0; i < tableList.size(); i++) {
try {
tableList.get(i).commitLock();
} catch (Exception e) {
for (int j = i - 1; j >= 0; j--) {
tableList.get(i).commitUnlock(); // should be get(j)
}
}
}
}
```
Consequences on the failure path:
- Commit locks already acquired for tables `0..i-1` are never released (leak).
- The failing table (`i`) is `commitUnlock()`-ed repeatedly on a lock it does not hold.
- The exception is swallowed and the outer loop keeps trying to lock the remaining tables.
The correct siblings `tryCommitLockTables` and `writeLockTablesOrMetaException` use `get(j)` and propagate the failure.
Note: in normal operation `commitLock()` → `MonitoredReentrantLock.lock()` does not throw, so this `catch` is defensive and the bug is latent; the fix makes the rare failure path correct.
### What You Expected?
On a commit-lock acquisition failure, the locks already acquired should be released (`get(j)`) and the failure propagated, instead of leaking locks and silently continuing.
### How to Reproduce?
Code inspection: `commitLockTables` unlocks `get(i)` instead of `get(j)` in its rollback loop, diverging from every other lock helper in the class. A unit test that injects a table whose `commitLock()` throws shows the previously-locked table is left locked.
### Anything Else?
Fix proposed in PR #64676.
### Are you willing to submit PR?
- [x] Yes I am willing to submit a PR!
Contributor guide
Research direction
Start in MetaLockUtils.commitLockTables and compare its rollback loop with tryCommitLockTables and writeLockTablesOrMetaException. Use a unit test with a table whose commitLock() throws; done means previously acquired tables are unlocked and the failure is propagated rather than swallowed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100