DEV Community

Nikhil Kamani
Nikhil Kamani

Posted on

2 Concurrency Traps That Look Thread-Safe But Aren't


I have 13 years of Java experience and have interviewed hundreds of developers at MNCs. Both of these come up constantly in multithreading rounds — and both involve code that looks perfectly safe until you think about it for another 10 seconds.


Q1: Is this thread-safe?

private volatile int count = 0;

public void increment() {
    count++;
}
Enter fullscreen mode Exit fullscreen mode

Answer: No.

Reason, in plain terms: volatile guarantees visibility — every read and write goes straight to main memory, so all threads see the latest value immediately. It does not guarantee atomicity. count++ is actually three separate steps: read the value, add 1, write it back. volatile makes each of those three steps visible to other threads, but it doesn't stop another thread from jumping in between them.

Thread-1: reads count (0)
Thread-2: reads count (0)      <- interleaves here
Thread-1: writes count = 1
Thread-2: writes count = 1     <- overwrites Thread-1's update

Result: count = 1 (should be 2)
Enter fullscreen mode Exit fullscreen mode

This is why a "thread-safe" counter silently loses updates under load, and it's almost impossible to catch in testing — it only shows up under real concurrent traffic.

Fix: use AtomicInteger, which provides both visibility and atomicity:

private final AtomicInteger count = new AtomicInteger(0);

public void increment() {
    count.incrementAndGet();
}
Enter fullscreen mode Exit fullscreen mode

Q2: Is this safe on a ConcurrentHashMap?

ConcurrentHashMap<String, Integer> map = new ConcurrentHashMap<>();

if (!map.containsKey("user1")) {
    map.put("user1", 1);
}
Enter fullscreen mode Exit fullscreen mode

Answer: No.

Reason: every individual operation on ConcurrentHashMap is thread-safe — containsKey() alone is safe, put() alone is safe. But the sequence of the two together is not one atomic unit. Another thread can insert "user1" in the gap between your containsKey() check and your put() call, and you'll silently overwrite it.

Fix: use the map's built-in atomic compound methods instead of check-then-act:

map.putIfAbsent("user1", 1);                        // atomic check-and-put
map.compute("key", (k, v) -> v == null ? 1 : v + 1); // atomic get-modify-put
map.merge("key", 1, Integer::sum);                   // atomic get-modify-put
Enter fullscreen mode Exit fullscreen mode

I cover this and more (Core Java, Java 8 to 21, Multithreading, Spring Boot, Microservices, Design Patterns, Coding Round Patterns) in my guide.

Free sample: https://drive.google.com/file/d/1u3PQbTY1gLn34UmWG7Cxx4cmdibD2dvU/view?usp=sharing

Full guide: https://kamaninikhil.gumroad.com/l/java-interview-guide

Feedback, typos, or suggestions? Email me: kamaninikhil71@gmail.com

Which one would you have caught in a code review? Drop it in the comments.

Top comments (2)

Collapse
 
speed_engineer profile image
speed engineer

Both of these are really the same root cause, and it's worth naming out loud in an interview: the individual operation is atomic, but the compound action isn't. volatile makes each read and write visible, but count++ is read-modify-write (three ops); ConcurrentHashMap makes each call atomic, but containsKey-then-put is two calls. Any time you "read something, then act on the result," ask whether the read-and-act is one atomic unit, because by default it isn't.

Two senior follow-ups interviewers like to push on after these:

On Q1: AtomicInteger fixes one counter, but atomics don't compose. If two values have to stay consistent with each other (a balance and its ledger entry, or two counters that must sum to a constant), making each one atomic doesn't make the pair atomic; a thread can observe one updated and the other not. There you're back to a lock, or a single AtomicReference to an immutable snapshot holding both.

On Q2: compute / computeIfAbsent / merge close the race, but the remapping function runs under the bin's lock, so it must be short and side-effect-free, and it must not touch the same map from inside the lambda. In Java 9+ computeIfAbsent actually throws on recursive self-modification, and you can deadlock if the function reaches for another lock. Perfect for "increment this count," dangerous the moment someone stuffs real business logic in there.

Collapse
 
nikz11 profile image
Nikhil Kamani

This is a great follow-up — both points are exactly right, and worth expanding on.

On the AtomicReference point:
This can be sorted with AtomicReference:

record Account(int balance, int ledgerCount) {}

AtomicReference<Account> account = new AtomicReference<>(new Account(1000, 0));

public void transfer(int amount) {
    Account current, updated;
    do {
        current = account.get();
        updated = new Account(current.balance() - amount, current.ledgerCount() + 1);
    } while (!account.compareAndSet(current, updated));
}
Enter fullscreen mode Exit fullscreen mode

Bundle the related values into one immutable object, and swap the whole object atomically instead of updating two separate atomics. Either both fields change together, or the swap fails and retries — no in-between state for another thread to see.

However, synchronized is the right default here — AtomicReference is a specialized tool for when you specifically need to avoid blocking (latency-sensitive, high-contention, cheap+pure update logic), not a strictly-better replacement.