How would you design a thread-safe component, and why is adding synchronized to every method not a design?

Write the policy down before you write a keyword

A thread-safe component is one whose invariants hold under concurrent access without callers doing anything special. Getting there is a bookkeeping exercise before it is a coding one: list every piece of mutable state the component owns, and assign each one exactly one of three policies.

Confined state never escapes to another thread, so it needs nothing. A local variable, or an object created and discarded inside one method, is confined by construction. A field is confined only if you can show nothing publishes it, which is why returning it from a getter breaks the policy silently.

Immutable state is safe to share because nobody can change it. In Java that means final fields holding values that are themselves immutable, and it removes the need for any lock on the read path — the strongest lever in the whole design.

Guarded state is mutable and shared, so every read and every write happens while holding one specific, named lock. The word "specific" is the load-bearing part: a reviewer must be able to say which lock protects which field, and if you cannot write that sentence, the component is not thread-safe, it is merely hard to break by hand.

Saying which of the three applies to each field is the answer interviewers are listening for, and it takes about thirty seconds. Almost everything that follows is mechanical once it is stated.

Push as much as possible into the immutable column

The reason to prefer immutability is not elegance. It is that readers of immutable state need no coordination at all, so on a read-heavy component the contention disappears rather than being managed.

The technique that gets you there when the state genuinely changes is snapshot-and-replace: gather the mutable fields into one immutable value object, hold a single reference to it, and publish a whole new value on every change. Readers take one reference and are then working on a stable snapshot, immune to anything a writer does next. A hot configuration reload is the everyday case — every consumer of the config reads a coherent version rather than a half-updated one.

The subtlety is that the reference itself must be published safely. A plain field can be seen stale by another thread indefinitely; volatile gives you visibility, and an AtomicReference gives you visibility plus a compare-and-set so two writers cannot lose an update.

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicReference;

/** Per-key token bucket. Every bucket's state is immutable; the only mutation
 *  is an atomic swap of the reference, so there is no lock anywhere. */
public final class TokenBucketLimiter {

    private record Bucket(double tokens, long lastNanos) {}

    private final double capacity;
    private final double refillPerSecond;
    private final Map<String, AtomicReference<Bucket>> buckets = new ConcurrentHashMap<>();

    public TokenBucketLimiter(double capacity, double refillPerSecond) {
        this.capacity = capacity;
        this.refillPerSecond = refillPerSecond;
    }

    public boolean tryAcquire(String key) {
        AtomicReference<Bucket> ref = buckets.computeIfAbsent(
                key, k -> new AtomicReference<>(new Bucket(capacity, System.nanoTime())));

        while (true) {
            Bucket current = ref.get();
            long now = System.nanoTime();          // monotonic; a wall clock would go backwards
            double refilled = Math.min(capacity,
                    current.tokens() + (now - current.lastNanos()) / 1e9 * refillPerSecond);

            if (refilled < 1.0) return false;      // refused, and nothing was published

            // If another thread got here first, compareAndSet fails and we recompute
            // from its value. That retry is why no token can be issued twice.
            if (ref.compareAndSet(current, new Bucket(refilled - 1.0, now))) return true;
        }
    }
}

Two things about that design are worth saying out loud in an interview. Contention is per key, so two customers never block each other, and there is no lock to forget to release or to order wrongly. And the retry loop is bounded in practice by contention on a single key, which for a rate limiter is exactly the case you are willing to spend a retry on.

Granularity follows the invariants, not your taste

The rule that produces the right lock structure is short: fields that participate in the same invariant must be guarded by the same lock, and fields that do not should not be.

If a bank account holds a balance and an overdraft limit and the invariant is balance >= -limit, those two fields are one unit and must move under one lock; guarding them separately gives you two atomic operations and a window between them in which the invariant is false. Conversely, if a component maintains a request counter and an unrelated cache of user names, one lock over both means every counter increment blocks every cache read for no reason at all.

That gives you the three standard shapes. One lock per object when all the state is one invariant, which is most small components. One lock per independent group of fields when there are genuinely several. And striping — a lock per key, or per bucket — when the state is a large collection of independent entries, which is what ConcurrentHashMap does internally and what the limiter above does with one atomic reference per key.

Prefer the primitive that already exists. ConcurrentHashMap.computeIfAbsent gives you atomic get-or-create; AtomicLong or LongAdder gives you counters; a BlockingQueue gives you the producer-consumer handoff. Hand-rolling these is where real bugs come from, and choosing one signals that you know the library rather than that you can spell synchronized. The one caveat worth mentioning is that computeIfAbsent and compute run your function while holding a lock on that bin, so the function must be short and must not touch the same map.

Why sprinkling synchronized is not a design

Adding the keyword to every method makes each individual method atomic, which sounds like the goal and is not, for four separate reasons.

Atomicity does not compose. This is the central point, and the one that separates candidates. Two atomic calls in sequence give you a non-atomic sequence, and the gap between them is a race no amount of method-level locking closes.

import java.util.HashMap;
import java.util.Map;

final class Counters {
    private final Map<String, Integer> counts = new HashMap<>();

    public synchronized int get(String key) { return counts.getOrDefault(key, 0); }
    public synchronized void set(String key, int value) { counts.put(key, value); }

    // Both calls are synchronised. This method is still a lost-update race,
    // because another thread can run between the get and the set.
    void incrementBadly(String key) { set(key, get(key) + 1); }

    // Correct: the read and the write are inside one critical section.
    synchronized void increment(String key) { counts.merge(key, 1, Integer::sum); }
}

Every check-then-act and read-modify-write has this shape, which is why Collections.synchronizedMap does not make your code thread-safe — it makes each map operation thread-safe, and your logic is almost always a sequence of them.

It says nothing about what is protected. synchronized on an instance method locks this, which is a lock any external code holding a reference to your object can also acquire, and which protects whatever the method body happens to touch. A reader cannot tell from the keyword which invariant is being maintained. A private final Object lock with a comment naming the fields it guards carries the design; the keyword alone carries only a hope.

It serialises work that had no reason to contend. One lock per object is a throughput ceiling, and on a component with a hot read path it is the ceiling you will hit first. This is the cost people notice in production and never in a test.

It invites deadlock as soon as a method calls out. A synchronized method that invokes a listener, a callback or another locked object is holding your lock while running code you did not write, which may acquire locks in the opposite order or block indefinitely. The discipline is to do the state change under the lock, copy out what you need, release, and only then call the alien code — which is another reason immutable snapshots help, because the thing you copy out is already safe to hand over.

There is a fifth, quieter failure that the keyword cannot fix at all: a synchronised getter that returns a reference to internal mutable state. The method is atomic and the caller now has an unguarded handle on your fields, so every subsequent access happens outside your lock. Return an immutable copy or a value object instead. This is the same defect as leaking a collection from an encapsulated class, and concurrency turns it from a maintenance problem into a correctness one.

The sentence to be able to say for every field is "this is confined, or immutable, or guarded by this lock". A component whose author can say it is thread-safe by construction, and one whose author can only point at the keyword is thread-safe by luck.