apache / apache/druid

Make IntelliJ's inspection "Lock acquired but not safely unlocked" a error

Open
#8,449 1 comment 0 reactions 0 assignees View on GitHub
Area - Automation/Static Analysis Contributions Welcome Improvement
Dominant language
Java
Stars
14.1k
Forks
3.8k
Avg merge
2d 58m
Merged PRs (30d)
233

Description

There are about a dozen violations of this rule in the codebase. Most of them look relatively benign, though they are still antipatterns, for example, in `NamespaceLookupExtractorFactory`, where code
```java
final Lock writeLock = startStopSync.writeLock();
try {
writeLock.lockInterruptibly();
}
catch (InterruptedException e) {
throw new RuntimeException(e);
}
```
Doesn't make sense to me. I think it should be replaced with simple
```java
final Lock writeLock = startStopSync.writeLock();
writeLock.lock();
```

A little more serious violations are in several classes where:
- There are some statements between `lock()` method call and the beginning of the `try {}` block: exceptions in these statements may lead to a dangling lock.
- `lockInterruptibly()` is called within the `try {}` block, rather than before it: if the locking is interrupted, an attempt to unlock in the `finally {}` block will lead to an `IllegalMonitorStateException` (at best), masking the real problem.
- `fullyLock()` and `fullyUnlock()` in `BytesBoundedLinkedQueue` should be annotated `@LockMethod({"takeLock", "putLock"})` and `@UnlockMethod({"takeLock", "putLock"})`, respectively.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the IntelliJ inspection violations, including the locking code in NamespaceLookupExtractorFactory and the fullyLock() and fullyUnlock() methods in BytesBoundedLinkedQueue. Check each reported lock pattern and ensure the inspection no longer reports unsafe acquisition or release; completion includes the requested lock annotations where applicable.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.