Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright 2026 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.errorprone.annotations.concurrent;

import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.CLASS;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;

/**
* Indicates that the annotated functional-interface parameter is run synchronously on the calling
* thread before the method returns, and is not stored or passed to another thread.
*
* <p>The {@link GuardedBy} check relies on this: a lambda or method reference passed as this
* argument is analyzed as if the caller's locks are held.
*/
@Target(PARAMETER)
@Retention(CLASS)
public @interface RunsImmediately {}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package com.google.errorprone.bugpatterns.threadsafety;

import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.bugpatterns.threadsafety.HeldLockAnalyzer.INVOKES_LAMBDAS_IMMEDIATELY;
import static com.google.errorprone.matchers.Description.NO_MATCH;

import com.google.common.base.Joiner;
Expand Down Expand Up @@ -79,7 +78,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState state) {
var parent = state.getPath().getParentPath().getLeaf();
if (parent instanceof MethodInvocationTree methodInvocationTree
&& INVOKES_LAMBDAS_IMMEDIATELY.matches(methodInvocationTree, state)) {
&& HeldLockAnalyzer.invokesArgumentImmediately(methodInvocationTree, tree, state)) {
return NO_MATCH;
}
analyze(state.withPath(new TreePath(state.getPath(), tree.getBody())));
Expand All @@ -90,7 +89,7 @@ public Description matchLambdaExpression(LambdaExpressionTree tree, VisitorState
public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
var parent = state.getPath().getParentPath().getLeaf();
if (parent instanceof MethodInvocationTree methodInvocationTree
&& INVOKES_LAMBDAS_IMMEDIATELY.matches(methodInvocationTree, state)) {
&& HeldLockAnalyzer.invokesArgumentImmediately(methodInvocationTree, tree, state)) {
return NO_MATCH;
}
analyze(state);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCExpression;
import com.sun.tools.javac.tree.JCTree.JCNewClass;
Expand All @@ -65,7 +67,7 @@
*/
public final class HeldLockAnalyzer {
/** Methods which invoke lambdas on the same thread. */
static final Matcher<ExpressionTree> INVOKES_LAMBDAS_IMMEDIATELY =
private static final Matcher<ExpressionTree> INVOKES_LAMBDAS_IMMEDIATELY =
anyOf(
instanceMethod()
.onExactClass("java.util.Optional")
Expand All @@ -88,6 +90,32 @@ public final class HeldLockAnalyzer {
.onClass("com.google.common.collect.Iterables")
.namedAnyOf("tryFind", "any", "all", "indexOf"));

private static final String RUNS_IMMEDIATELY =
"com.google.errorprone.annotations.concurrent.RunsImmediately";

/**
* Returns true if {@code functionalArgument} (a lambda or method reference) passed to {@code
* invocation} is run on the same thread, i.e. the method is one of {@link
* #INVOKES_LAMBDAS_IMMEDIATELY} or the matching parameter is {@code @RunsImmediately}.
*/
static boolean invokesArgumentImmediately(
MethodInvocationTree invocation, ExpressionTree functionalArgument, VisitorState state) {
if (INVOKES_LAMBDAS_IMMEDIATELY.matches(invocation, state)) {
return true;
}
MethodSymbol sym = ASTHelpers.getSymbol(invocation);
if (sym == null) {
return false;
}
List<VarSymbol> params = sym.getParameters();
int index = invocation.getArguments().indexOf(functionalArgument);
if (index < 0 || params.isEmpty()) {
return false;
}
VarSymbol param = index < params.size() ? params.get(index) : params.getLast();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic specifically supports varargs, which is the case where index >= params.size()

return ASTHelpers.hasAnnotation(param, RUNS_IMMEDIATELY, state);
}

/** Listener interface for accesses to guarded members. */
public interface LockEventListener {

Expand Down Expand Up @@ -231,7 +259,7 @@ public Void visitNewClass(NewClassTree tree, HeldLockSet locks) {
public Void visitLambdaExpression(LambdaExpressionTree node, HeldLockSet heldLockSet) {
var parent = getCurrentPath().getParentPath().getLeaf();
if (parent instanceof MethodInvocationTree methodInvocationTree
&& INVOKES_LAMBDAS_IMMEDIATELY.matches(methodInvocationTree, visitorState)) {
&& invokesArgumentImmediately(methodInvocationTree, node, visitorState)) {
return super.visitLambdaExpression(node, heldLockSet);
}
// Don't descend into lambdas; they will be analyzed separately.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,162 @@ public synchronized void add(Optional<String> x) {
.doTest();
}

@Test
public void runsImmediately_lambda() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import com.google.errorprone.annotations.concurrent.GuardedBy;
import com.google.errorprone.annotations.concurrent.RunsImmediately;

class Test {
@GuardedBy("this")
private int state = 0;

public synchronized void modifyState(int newState) {
safeRun(() -> modifyStateInternal(newState));
}

@GuardedBy("this")
private void modifyStateInternal(int newState) {
state = newState;
}

private void safeRun(@RunsImmediately Runnable runnable) {
runnable.run();
}
}
""")
.doTest();
}

@Test
public void runsImmediately_methodReference() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import com.google.errorprone.annotations.concurrent.GuardedBy;
import com.google.errorprone.annotations.concurrent.RunsImmediately;

class Test {
@GuardedBy("this")
private int state = 0;

public synchronized void modifyState() {
safeRun(this::zeroStateInternal);
}

@GuardedBy("this")
private void zeroStateInternal() {
state = 0;
}

private void safeRun(@RunsImmediately Runnable runnable) {
runnable.run();
}
}
""")
.doTest();
}

@Test
public void runsImmediately_lambda_synchronizedBlock() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import com.google.errorprone.annotations.concurrent.GuardedBy;
import com.google.errorprone.annotations.concurrent.RunsImmediately;

class Test {
private final Object lock = new Object();

@GuardedBy("lock")
private int state = 0;

public void modifyState(int newState) {
synchronized (lock) {
safeRun(() -> state = newState);
}
}

private void safeRun(@RunsImmediately Runnable runnable) {
runnable.run();
}
}
""")
.doTest();
}

@Test
public void runsImmediately_lambda_wrongGuard() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import com.google.errorprone.annotations.concurrent.GuardedBy;
import com.google.errorprone.annotations.concurrent.RunsImmediately;

class Test {
private final Object lock = new Object();

@GuardedBy("lock")
private int state = 0;

public synchronized void modifyState(int newState) {
// BUG: Diagnostic contains: should be guarded by 'this.lock'
safeRun(() -> state = newState);
}

private void safeRun(@RunsImmediately Runnable runnable) {
runnable.run();
}
}
""")
.doTest();
}

@Test
public void runsImmediately_lambda_multipleGuardedAccesses() {
compilationHelper
.addSourceLines(
"Test.java",
"""
import com.google.errorprone.annotations.concurrent.GuardedBy;
import com.google.errorprone.annotations.concurrent.RunsImmediately;

class Test {
@GuardedBy("this")
private int a = 0;

@GuardedBy("this")
private int b = 0;

private final Object other = new Object();

@GuardedBy("other")
private int c = 0;

public synchronized void modify() {
safeRun(
() -> {
a = 1;
b = 2;
// BUG: Diagnostic contains: should be guarded by 'this.other'
c = 3;
});
}

private void safeRun(@RunsImmediately Runnable runnable) {
runnable.run();
}
}
""")
.doTest();
}

@Test
public void methodReferences_shouldBeFlagged() {
compilationHelper
Expand Down
27 changes: 27 additions & 0 deletions docs/bugpattern/GuardedBy.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,33 @@ private void doSomething(Runnable r) {
However, the check does special-case some method calls which are known to
immediately call the provided lambda or method reference.

For your own methods, you can opt in to the same behavior by annotating the
functional-interface parameter with
`com.google.errorprone.annotations.concurrent.RunsImmediately`. This documents
that the argument, if it is invoked at all, is invoked synchronously on the
calling thread before the method returns, so a lambda or method reference passed
there is analyzed in the caller's lock scope:

```java
class Transaction {
@GuardedBy("this")
int x;

public synchronized void handle() {
doSomething(() -> {
x++; // OK: 'doSomething' runs the lambda immediately, while 'this' is held.
});
}

private void doSomething(@RunsImmediately Runnable r) {
r.run();
}
}
```

The contract is trusted but not verified: annotating a parameter whose value is
actually deferred to another thread can hide real concurrency bugs.

#### False negatives with aliasing

```java
Expand Down
Loading