Classic mistakes with Java concurrency stress-tested and statically checked

INTRODUCTION

Rationale

There are only three hard computer science problems: cache invalidation, naming things, concurrency and off-by-one errors. In this post, we’ll cover the basics of concurrency. Every Java developer is familiar with the seminal book Java Concurrency In Practice (JCIP) ¹. A corpus of thread safety mistakes from there will be presented here as hands-on examples supplied with practical JVM stress testing and static code analysis.. This allowed me - and the people who might clone the repo - to see each thread safety mistake with much greater detail. Stress tests reproduce the failure modes, and the static code analysis tools demonstrate which concurrency bugs are easily visible to compilers and which ones remain hidden.

Goals

For each of about 7 examples on Java concurrency errors adapted from JCIP, show:

• If the code indeed breaks on x86 CPU, demonstrate this by jcstress framework.
• If the lightweight static analysis (Spotbugs, earlier Findbugs) fails your build.
• If a heavy, compiler-connected static analysis #2 (ErrorProne) flags the architectural anti-pattern.

Non-goals

• Exploring modern patterns using java.util.concurrent and later tools (e.g. fork-join) to keep the article scope focused.
• Explaining HotSpot and CPU behavior for each problem in-depth.

SETUP DESCRIPTION

Java environment

I have used Java 21 with Maven 3+. JCStress didn’t work for Java 25 (yet) so I had to downgrade. Maven because I’m more familiar with that than with Gradle. I’ve set up each package to contain the example together with its stress-testing class to simplify navigation. Package net.jcip.annotations was used to document thread (un)safety, which was a natural decision - after all, this project is based on JCIP head to toes. I also included JSpecify (which anyway comes bundled with Spring Boot) @NonNull annotation, which is useful with Intellij IDEA inspections.

Static code analysis

As building threadsafe code is hard, I thought that static code analysis tools could give me some support. There are tools that connect to your javac compiler, and that don’t. Spotbugs ², a successor of popular Findbugs, is an abstract-syntax-tree scanner that runs on compiled bytecode, doesn’t require a complex setup and can be made to fail your builds. ErrorProne ³ from Google, endorsed by Java champions, is significantly harder to setup because it requires changing visibility of some javac packages.

JCStress - multi-JVM stress testbed

As proving that concurrent code is broken requires some lucky timings, you need a test software. JCStress ⁴ is a formidable program that allows you to catch rare thread interleaving behaviors with ease. For each @State annotation, each @Actor runs exactly once per test. You inject I_Result instances to @Actor or @Arbiter to modify its r1 field that connects to @Outcome annotation’s id, i.e. r1 = -1 means the test for concurrency problems is positive. Beware that JCStress manages its own Threads. Therefore, some JCIP book examples that used Threads had to be significantly modified. As well, I had to get rid of static fields in some other examples (especially when initialising fields), because else I would get just the first test run to be meaningful, out of thousands.

EXAMPLES

1. NonAtomicCheckThenAct

A classic race condition: a thread check a precondition, then another thread modifies a precondition so it’s no longer valid, so the first thread acts on wrong data. Easily caught by jcstress, but for some reason static code analysis ignores that. Of course, replacing int count with AtomicInteger is way to fix this.

/**
 * Adapted from JCIP 2.2
 * Non-atomic compound actions on a shared counter;
 */
@NotThreadSafe
public class RacyCounter {
    private int count;
    public void increment() {
        count = count + 1;
    }
    public int getCount() {
        return count;
    }
}

2. NoVisibility

A classic problem with publicating a variable. Changes are seen by one thread (i.e. the value gets to the local CPU store), but never seen by another. While a volatile keyword establishes the necessary happens-before relationship, its omission here yields unpredictable results.

/**
 * adapted from JCIP 3.1
 * if number is set before ready, jcstress tests pass on x86 CPUs
 * on ARM CPUs non-volatile state variables are unsafe
 */
@NotThreadSafe
public class EasyNoVisibility {
    private boolean ready = false;
    private int number;

    public int getNumber(){
        while (!ready) {
            Thread.yield(); // ErrorProne: Relying on the thread scheduler is discouraged.
        }
       return number;
    }

    /**
     * Stores are not reordered with other stores
     */
    public void ready()  {
        ready = true;
        number = 42;
    }
}

3. Faulty Poly Invariant

Like the first example, another race condition, but now our state is multi-variable with an invariant to protect (one variable always less or equal than another). Here the volatile keyword doesn’t work because we need a unified lock to make changes across both state variables atomic.

/**
 * Adapted from JCIP 4.10
 * To protect the invariant, the variables must be updated atomically.
 */
@NotThreadSafe
public class FaultyPolyInvariant {

    /** Invariant: lower <= upper */
    @GuardedBy("this")
    private volatile int lower = 0;
    @GuardedBy("this")
    private volatile int upper = 10;

    public void setLowerBound(int newLower) {
        if (newLower > upper) {
            throw new IllegalArgumentException("newLower > upper");
        }
        this.lower = newLower;
    }

    public void setUpperBound(int newUpper) {
        if (newUpper < lower) {
            throw new IllegalArgumentException("newUpper < lower");
        }
        this.upper = newUpper;
    }

    public boolean isOK() {
        return lower <= upper;
    }
}

4. Incomplete locking

This is not a JCIP example, but a demonstration that sometimes, relatively easy-spotted mistakes look completely normal and also invisible to the static code analysis. The variable guarding the invariant wasn’t completely locked - and yet ErrorProne or SpotBugs ignore it. Even setting SpotBugs to check with maximal effort didn’t catch this mistake.

/**
 * Non-JCIP example.
 * A state variable is accessible around its lock.
 * Interesting that nor ErrorProne nor SpotBugs find this.
 */
@NotThreadSafe
public class IncompleteLocking {

    private final Object lock = new Object();

    @GuardedBy("lock")
    private StringBuilder ledger = new StringBuilder("0");

    public void record(String entry) {
        synchronized (lock) {
            ledger.append('|').append(entry);
        }
    }

    /** Intentionally reads a guarded field without holding the lock. */
    public String peekUnsafe() {
        return ledger.toString();
    }
}

5. Locking on wrong object

This is inspired by a Java SE certification test. You add synchronization, but in wrong place, still leaving the mutable state free to be modified randomly.

/**
 * Non-JCIP example inspired by Oracle Java SE exam
 * Pattern when the state seems locked, but on the wrong class!
 */
@ThreadSafe
public class WrongLock {

    private int total;

    public int getTotal() {
        return total;
    }

    public void increment(){
        total++;
    }
}

/**
 * Enough to have more than one WrongLockRunner and call increment() concurrently to get a race condition!
 */
public class WrongLockRunner {
    public synchronized void increment(WrongLock wl )  {
        wl.increment();
    }
    public synchronized int get(WrongLock wl )  {
        return wl.getTotal();
    }
}

6. Escaping Object

One of reasons why Java loves static factory methods. Static factory methods make it much harder to give this reference to a place that might use it too early (unsafe publication idiom):

/**
 * Adapted from JCIP 3.7
 * A this reference gets published before the object is in consistent state.
 */
@NotThreadSafe
public class EscapingObject {
    public final UUID id;

    public EscapingObject(EscapingContext context) {
        context.activeInstance = this;
        this.id = new UUID(64l, 0l);
    }
}

7. *Bizarre example on possible reordering;

Testing this one proved elusive because it relies on Threads. The main point was that it’s possible for the program to print (0,0) due to thread reordering, because “each thread has no dataflow dependency on each other”, so I’m including the original code here:

/**
* Original JCIP listing 16.1
**/
public class PossibleReordering {
    static int x = 0, y = 0;
    static int a = 0, b = 0;
    public static void main(String[] args)
        throws InterruptedException {
    Thread one = new Thread(new Runnable() {
        public void run() {
      a = 1;
      x = b;
    }
  });
  Thread other = new Thread(new Runnable() {
    public void run() {
      b = 1;
      y = a;
    }
  });
  one.start(); other.start();
  one.join();  other.join();
  System.out.println("( "+ x + "," + y + ")");
    }
}

As explained above, Threads don’t work well with jstress tests. As well, I hard to make the code return its answer, and make the fields non-static. Still, as far as we’re concerned with the statement reordering, it’s possible to do runA(), runB() and peek() in the client code that would get reordered so peek() hits a state where both x and y are zero.

/**
 * Adapted from JCIP listing 16.1
 */
@NotThreadSafe
public class EasyPossibleReordering {
    public record IntPair(int a, int b){};
    private int a = 0, b = 0;
    private int x = 0, y = 0;

    public void runA(){
        a = 1;
        x = b;
    }

    public void runB(){
        b = 1;
        y = a;
    }

    public IntPair peek(){
        return new IntPair(x, y);
    }
}

APPENDIX

A: Several lines from JCIP page 1 summary:

• All concurrency issues boil down to coordinated access to mutable state.
• Guard all variables in an invariant with the same lock
• A program that accesses a mutable variable from multiple threads without synchronization is broken;

B: The accompanying code is available at:

GitHub

C: URLs: