google / google/error-prone

Autocloseable support in GuardedByChecker

Open
#4,207 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

Currently, `GuardedByChecker` reports false positives if a lock is being held in a try-with-resources block. There's a TODO in the codebase for this, but I didn't see any tickets for this issue so I thought I'd start a discussion.

https://github.com/google/error-prone/blob/7acf133dc9fd0366afb57ee8e4582a78c808b73d/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/HeldLockAnalyzer.java#L205-L206

When JDK 7 introduced the try-with-resources block, there was a lot of discussion about whether various locks should be supported. Eventually, it was decided that it would not be supported in the standard Java class library; however, the choice was controversial enough that I'm sure many people are using one of the many third party libraries to solve this or rolling their own Autoclosable solution. Would it be possible to add a heuristic to detect these AutoCloseable wrappers? Maybe by allowing users to explicitly annotate them?

Maybe something that looks like this?

```java
class Example {

private final Lock lock = new Lock();

@GuardedBy("lock")
private int counter;

void test() {
try (var u = LockUtil.lock(lock)) {
++counter;
}
}
}

class LockUtil {

@MustBeClosed
@LockHolder("delegate") // Informs HeldLockAnalyzer to add 'delegate' to heldset. Maybe even verify unlock is called by close?
public static AutoCloseable lock(Lock delegate) {
delegate.lock();
return delegate::unlock;
}
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.