Multithreading

AvoidSynchronizedAtMethodLevel

Since: PMD 3.0

Priority: Medium (3)

Method-level synchronization will pin virtual threads and can cause performance problems. Additionally, it can cause problems when new code is added to the method. Block-level ReentrantLock helps to ensure that only the code that needs mutual exclusion will be locked.

See also AvoidSynchronizedStatement.

This rule is defined by the following XPath expression:

//MethodDeclaration[pmd-java:modifiers() = "synchronized"]

Example(s):

public class Foo {
    // Try to avoid this:
    synchronized void foo() {
        // code, that doesn't need synchronization
        // ...
        // code, that requires synchronization
        if (!sharedData.has("bar")) {
            sharedData.add("bar");
        }
        // more code, that doesn't need synchronization
        // ...
    }

    // Prefer this:
    Lock instanceLock = new ReentrantLock();

    void bar() {
        // code, that doesn't need synchronization
        // ...
        instanceLock.lock();
        try {
            // code, that requires synchronization
            if (!sharedData.has("bar")) {
                sharedData.add("bar");
            }
        } finally {
            instanceLock.unlock();
        }
        // more code, that doesn't need synchronization
        // ...
    }

    // Or prefer this with tryLock:
    Lock instanceLock = new ReentrantLock();

    void bar() {
        // code, that doesn't need synchronization
        // ...
        if (instanceLock.tryLock(10, TimeUnit.SECONDS)) {
            try {
                // code, that requires synchronization
                if (!sharedData.has("bar")) {
                    sharedData.add("bar");
                }
            } finally {
                instanceLock.unlock();
            }
        } else {
            // unable to acquire the lock
        }
        // more code, that doesn't need synchronization
        // ...
    }

// Try to avoid this for static methods:
    static synchronized void fooStatic() {
    }

    // Prefer this:
    private static Lock CLASS_LOCK = new ReentrantLock();

    static void barStatic() {
        // code, that doesn't need synchronization
        // ...
        CLASS_LOCK.lock();
        try {
            // code, that requires synchronization
        } finally {
            CLASS_LOCK.unlock();
        }
        // more code, that doesn't need synchronization
        // ...
    }
}

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/AvoidSynchronizedAtMethodLevel" />

AvoidSynchronizedStatement

Since: PMD 7.5.0

Priority: Medium (3)

Reports any synchronization statements.

Virtual threads (introduced by Java 21 via JEP 444) are pinned to their carrier thread when executing code protected by synchronized (either synchronized blocks or synchronized methods). When the virtual threads are blocked by I/O and need to wait, the carrier thread stays unavailable for other virtual threads. If this happens with all carrier threads, no new virtual threads can be executed and this can cause performance problems.

With Java 24 (JEP 491) virtual threads can release their carrier thread, when they are blocked by I/O inside a synchronized block. The situation, that all carrier threads are exhausted, should be much less likely.

Note: Thread pinning still occurs, if the virtual thread is calling native methods inside a synchronized block.

This rule is defined by the following XPath expression:

//SynchronizedStatement

Example(s):

public class Foo {
    // Try to avoid this:
    void foo() {
        // code that doesn't need mutual exclusion
        synchronized(this) {
            // code that requires mutual exclusion
        }
        // more code that doesn't need mutual exclusion
    }

    // Prefer this:
    Lock instanceLock = new ReentrantLock();
    void foo() {
        // code that doesn't need mutual exclusion
        instanceLock.lock();
        try {
            // code that requires mutual exclusion
        } finally {
            instanceLock.unlock();
        }
        // more code that doesn't need mutual exclusion
    }

    // Or prefer this with tryLock:
    Lock instanceLock = new ReentrantLock();
    void foo() {
        // code that doesn't need mutual exclusion
        if (instanceLock.tryLock(10, TimeUnit.SECONDS)) {
            try {
                // code that requires mutual exclusion
            } finally {
                instanceLock.unlock();
            }
        } else {
            // unable to acquire the lock
        }
        // more code that doesn't need mutual exclusion
    }
}

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/AvoidSynchronizedStatement" />

AvoidThreadGroup

Since: PMD 3.6

Priority: Medium (3)

Avoid using java.lang.ThreadGroup; although it is intended to be used in a threaded environment it contains methods that are not thread-safe.

This rule is defined by the following XPath expression:

//ConstructorCall/ClassType[pmd-java:typeIs('java.lang.ThreadGroup')]
| //MethodCall[@MethodName = 'getThreadGroup']

Example(s):

public class Bar {
    void buz() {
        ThreadGroup tg = new ThreadGroup("My threadgroup");
        tg = new ThreadGroup(tg, "my thread group");
        tg = Thread.currentThread().getThreadGroup();
        tg = System.getSecurityManager().getThreadGroup();
    }
}

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/AvoidThreadGroup" />

AvoidUsingVolatile

Since: PMD 4.1

Priority: Medium High (2)

The volatile modifier is an extremely low-level concurrency tool (like notify and wait). It makes sure that all threads see a consistent value for a field, and it establishes a happens-before relationship: everything the writing thread did before the volatile write becomes visible to any thread that afterwards reads the field.

What volatile does not do:

• It doesn’t make updates atomic. Read and write remain separate operations, so counter++ is a read, an increment and a write - two threads can interleave and lose an update. The same applies to every check-then-act idiom.
• On a reference or array field, it only covers the reference itself. Writes to the referenced object’s fields, or to the array’s elements, are not covered.

Using volatile correctly requires more knowledge of the Java Memory Model than most Java developers have. Therefore, most Java developers shouldn’t use it. Instead, use the higher-level classes from java.util.concurrent, especially the Atomic wrapper classes:

• instead of a volatile boolean, int, long or reference: AtomicBoolean, AtomicInteger, AtomicLong, AtomicReference
• instead of a volatile array: AtomicIntegerArray, AtomicLongArray, AtomicReferenceArray
• for counters under contention: LongAdder, DoubleAdder
• for a value that never changes after construction: final
• for shared collections: the concurrent collections, e.g. ConcurrentHashMap

A related trap concerns 64-bit fields: a write to a non-volatile long or double field is not required to be atomic (JLS §17.7). The JVM may split it into two separate 32-bit writes, so another thread can read a value assembled from two different writes - a value nobody ever wrote. Declaring the field volatile does fix this specific problem, but it still leaves ++ and += non-atomic. AtomicLong fixes both and is the better default. (There is no AtomicDouble in the JDK; use DoubleAdder or an AtomicReference<Double>.)

Note that this rule allows no exceptions. This is intentional. Valid uses exist but are rare (e.g. DoubleCheckedLocking). If yours is one of them, suppress the violation explicitly with // NOPMD or @SuppressWarnings("PMD.AvoidUsingVolatile") and a comment explaining why.

This rule is defined by the following XPath expression:

//FieldDeclaration[pmd-java:modifiers() = "volatile"]

Example(s):

public class Data {
 private volatile String         var1; // not suggested
 private AtomicReference<String> var2; // preferred
}

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/AvoidUsingVolatile" />

DoNotUseThreads

Since: PMD 4.1

Priority: Medium (3)

The J2EE specification explicitly forbids the use of threads. Threads are resources, that should be managed and monitored by the J2EE server. If the application creates threads on its own or uses own custom thread pools, then these threads are not managed, which could lead to resource exhaustion. Also, EJBs might be moved between machines in a cluster and only managed resources can be moved along.

This rule is defined by the following XPath expression:

//ClassType
[pmd-java:typeIs('java.lang.Thread') or pmd-java:typeIs('java.util.concurrent.ExecutorService')]
(: allow Thread.currentThread().getContextClassLoader() :)
[not(parent::TypeExpression[parent::MethodCall[pmd-java:matchesSig('_#currentThread()')
                                               and parent::MethodCall[pmd-java:matchesSig('_#getContextClassLoader()')]
                                              ]
                           ]
)]
(: allow Thread.onSpinWait() :)
[not(parent::TypeExpression[parent::MethodCall[pmd-java:matchesSig('_#onSpinWait()')]])]
(: exclude duplicated types on the same line :)
 [not((parent::FieldDeclaration|parent::LocalVariableDeclaration)/VariableDeclarator/*[2][pmd-java:typeIs('java.lang.Thread') or pmd-java:typeIs('java.util.concurrent.ExecutorService')])
 or
  @BeginLine != (parent::FieldDeclaration|parent::LocalVariableDeclaration)/VariableDeclarator/ConstructorCall/ClassType/@BeginLine]
|
//MethodCall[*[1][not(pmd-java:nodeIs('MethodCall'))][pmd-java:nodeIs('Expression') and (pmd-java:typeIs('java.util.concurrent.Executors')
   or pmd-java:typeIs('java.util.concurrent.ExecutorService'))]]

Example(s):

// This is not allowed
public class UsingThread extends Thread {

}

// Neither this,
public class UsingExecutorService {

    public void methodX() {
        ExecutorService executorService = Executors.newFixedThreadPool(5);
    }
}

// Nor this,
public class Example implements ExecutorService {

}

// Nor this,
public class Example extends AbstractExecutorService {

}

// Nor this
public class UsingExecutors {

    public void methodX() {
        Executors.newSingleThreadExecutor().submit(() -> System.out.println("Hello!"));
    }
}

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/DoNotUseThreads" />

DontCallThreadRun

Since: PMD 4.3

Priority: Medium Low (4)

Explicitly calling Thread.run() method will execute in the caller’s thread of control. Instead, call Thread.start() for the intended behavior.

This rule is defined by the following XPath expression:

//MethodCall[ pmd-java:matchesSig("java.lang.Thread#run()") ]

Example(s):

Thread t = new Thread();
t.run();            // use t.start() instead
new Thread().run(); // same violation

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/DontCallThreadRun" />

DoubleCheckedLocking

Since: PMD 1.04

Priority: High (1)

Partially created objects can be returned by the Double Checked Locking pattern when used in Java. An optimizing JRE may assign a reference to the baz variable before it calls the constructor of the object the reference points to.

Note: With Java 5, you can make Double checked locking work, if you declare the variable to be volatile.

For more details refer to: http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html or http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

This rule is defined by the following Java class: net.sourceforge.pmd.lang.java.rule.multithreading.DoubleCheckedLockingRule

Example(s):

public class Foo {
    /*volatile */ Object baz = null; // fix for Java5 and later: volatile
    Object bar() {
        if (baz == null) { // baz may be non-null yet not fully created
            synchronized(this) {
                if (baz == null) {
                    baz = new Object();
                }
              }
        }
        return baz;
    }
}

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/DoubleCheckedLocking" />

NonThreadSafeSingleton

Since: PMD 3.4

Priority: Medium (3)

Creating singletons in a non-thread safe way leads to subtle, hard to reproduce concurrency issues: A singleton might sometimes be created more than once (so it is not really a singleton anymore) or calling code from a different thread might not see the singleton at all, or might see a non-null reference to an object that is not fully constructed yet.

See Effective Java, 3rd edition, item 78: "Synchronize access to shared mutable data".

The rule property checkNonStaticFields is false by default, so that only static fields are considered. This is the usual way to implement singletons. The rule finds methods (regardless of whether they are static or non-static) that write to static fields after a null-check.

If the property is set to true, then additionally non-static fields are considered. This allows to find lazy loading of instance fields.

Possible fixes for static singleton fields:

• Avoid lazy initialization if possible and instantiate the object directly, e.g. private static final Foo INSTANCE = new Foo();. Lazy initialization is an optimization you should only use if you have measured a performance gain (see Effective Java, 3rd edition, item 83: “Use lazy initialization judiciously”).
• Synchronize the entire method; for static fields, it must be a static synchronized method.
• Use an initialize-on-demand holder class. This is safe because the JVM initializes the holder class only once, on first access.
• Use an enum with a single item (see Effective Java, 3rd edition, item 3: "Enforce the singleton property with a private constructor or an enum type”). This is not possible, if the singleton has to extend a class other than Enum (interfaces are possible).
• Use AtomicReference with compareAndSet if a duplicate computation is acceptable.

Possible fixes for lazily initialized instance fields:

• Synchronize the entire method.
• Use AtomicReference with compareAndSet if a duplicate computation is acceptable.
• Use the double-checked locking pattern. The field must be declared volatile; without it, the Java Memory Model doesn’t guarantee that other threads see a fully constructed object. Note that this only works on Java 5 and later. (also possible for static fields, though the holder idiom is usually preferable) Reference

This rule is defined by the following Java class: net.sourceforge.pmd.lang.java.rule.multithreading.NonThreadSafeSingletonRule

Example(s):

private static Foo foo = null;

// multiple simultaneous callers may see partially initialized objects
public static Foo getInstance() {
    if (foo == null) {
        foo = new Foo();
    }
    return foo;
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/multithreading.xml/NonThreadSafeSingleton" />

Use this rule and customize it:

<rule ref="category/java/multithreading.xml/NonThreadSafeSingleton">
    <properties>
        <property name="checkNonStaticFields" value="false" />
    </properties>
</rule>

OverridingThreadRun

Since: PMD 7.24.0

Priority: Medium (3)

Overriding Thread::run method is not recommended. Instead, implement Runnable and pass an instance to the thread constructor. Using Runnable to represent a task makes it more reusable and allows your class to extend another class. When you use lambdas, this also allows you to write more concise code, e.g. new Thread(() -> System.out.println("Hello!")).start();.

This rule is defined by the following XPath expression:

//(AnonymousClassDeclaration|ClassDeclaration)[ pmd-java:typeIs('java.lang.Thread') ]/ClassBody
    /MethodDeclaration[@Name="run"][FormalParameters/@Size=0]

Example(s):

public class GreetingThread extends Thread {
    @Override
    public void run() {
        System.out.println("Hello!");
    }
}
new GreetingThread().start(); // not recommended, use Runnable instead

public class GreetingRunnable implements Runnable {
    @Override
    public void run() {
        System.out.println("Hello!");
    }
}
new Thread(new GreetingRunnable()).start(); // preferred

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/OverridingThreadRun" />

UnsynchronizedStaticFormatter

Since: PMD 6.11.0

Priority: Medium (3)

Instances of java.text.Format are generally not synchronized. Sun recommends using separate format instances for each thread. If multiple threads must access a static formatter, the formatter must be synchronized on block level.

This rule is defined by the following Java class: net.sourceforge.pmd.lang.java.rule.multithreading.UnsynchronizedStaticFormatterRule

Example(s):

public class Foo {
    private static final SimpleDateFormat sdf = new SimpleDateFormat();
    void bar() {
        sdf.format(); // poor, no thread-safety
    }
    void foo() {
        synchronized (sdf) { // preferred
            sdf.format();
        }
    }
}

This rule has the following properties:

Use this rule with the default properties by just referencing it:

<rule ref="category/java/multithreading.xml/UnsynchronizedStaticFormatter" />

Use this rule and customize it:

<rule ref="category/java/multithreading.xml/UnsynchronizedStaticFormatter">
    <properties>
        <property name="allowMethodLevelSynchronization" value="false" />
    </properties>
</rule>

UseConcurrentHashMap

Since: PMD 4.2.6

Priority: Medium (3)

Minimum Language Version: Java 1.5

Since Java5 brought a new implementation of the Map designed for multi-threaded access, you can perform efficient map reads without blocking other threads.

This rule is defined by the following XPath expression:

//VariableDeclarator[VariableId[pmd-java:typeIsExactly('java.util.Map')] and *[2][self::ConstructorCall and not(pmd-java:typeIs('java.util.concurrent.ConcurrentHashMap'))]]

Example(s):

public class ConcurrentApp {
  public void getMyInstance() {
    Map map1 = new HashMap();           // fine for single-threaded access
    Map map2 = new ConcurrentHashMap(); // preferred for use with multiple threads

    // the following case will be ignored by this rule
    Map map3 = someModule.methodThatReturnMap(); // might be OK, if the returned map is already thread-safe
  }
}

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/UseConcurrentHashMap" />

UseNotifyAllInsteadOfNotify

Since: PMD 3.0

Priority: Medium (3)

Thread.notify() awakens a thread monitoring the object. If more than one thread is monitoring, then only one is chosen. The thread chosen is arbitrary; thus it’s usually safer to call notifyAll() instead.

This rule is defined by the following XPath expression:

//MethodCall[@MethodName="notify" and ArgumentList[count(*) = 0]]

Example(s):

void bar() {
    x.notify();
    // If many threads are monitoring x, only one (and you won't know which) will be notified.
    // use instead:
    x.notifyAll();
  }

Use this rule by referencing it:

<rule ref="category/java/multithreading.xml/UseNotifyAllInsteadOfNotify" />