Your @Service Is a Singleton: Thread Safety in Spring

Here is a @Service that works perfectly in development and corrupts data in production.

@Service
public class ReceiptService {

    private final SimpleDateFormat formatter =
        new SimpleDateFormat("dd/MM/yyyy HH:mm");

    public String format(Sale sale) {
        return formatter.format(sale.getCreatedAt());
    }
}

Under one user it's fine. Under fifty, receipts start showing dates from other people's sales, and occasionally a date that never existed. No exception, no log line. Just wrong data.

One instance, every thread

Spring beans are singletons by default. @Service, @Component, @Repository — one instance created at startup, shared by every request for the lifetime of the application.

Your web server, meanwhile, handles requests on a pool of threads. So the real picture is: many threads, one object, and every field on that object is shared mutable state between all of them.

That's the entire problem. SimpleDateFormat happens to keep parsing state in a field internally, so two threads calling format() at the same time scribble over each other's work. It has been documented as not thread-safe since Java 1.1, and it is still the most common way to hit this.

The fix here is one word:

private static final DateTimeFormatter FORMATTER =
    DateTimeFormatter.ofPattern("dd/MM/yyyy HH:mm");

DateTimeFormatter is immutable and thread-safe by design. That's most of why java.time replaced the old API.

What is safe, and why

Local variables are always safe. Each thread has its own stack, so a variable declared inside a method exists once per call. This is why a stateless service handling a thousand concurrent requests needs no locking at all:

public String format(Sale sale) {
    // a new formatter per call — safe, just wasteful
    return new SimpleDateFormat("dd/MM/yyyy").format(sale.getCreatedAt());
}

Final fields holding immutable objects are safe. Your injected SaleRepository is fine — it's a stateless proxy. String, Integer, LocalDate, List.of(...) are all fine, because nobody can change them after construction.

Anything you can mutate is not safe. A counter, a HashMap cache, a StringBuilder, a partially-built object stored between method calls.

The rule I actually apply: a Spring bean should own dependencies, not data. If a field changes after startup, stop and think.

volatile is not a lock

This comes up in every interview and the wrong answer is common.

private volatile int salesCount;

public void record() {
    salesCount++; // still broken
}

volatile guarantees visibility: a write by one thread is immediately seen by others, instead of sitting in a CPU cache. It does not guarantee atomicity. salesCount++ is three operations — read, add one, write — and two threads can interleave between them, so both read 5 and both write 6. One sale disappears.

Use volatile when one thread writes a value and others just need to see it — a running flag for a background loop is the textbook case. When you're combining a read and a write, you need something stronger:

private final AtomicInteger salesCount = new AtomicInteger();

public void record() {
    salesCount.incrementAndGet(); // atomic
}

synchronized gives you both properties — mutual exclusion and visibility — but only one thread runs the block at a time, so it costs throughput. Reach for the atomic classes and the concurrent collections first; reach for synchronized when you need several fields to change together as one unit.

And the collection version of the same mistake:

private final Map<String, Product> cache = new HashMap<>();     // not safe
private final Map<String, Product> cache = new ConcurrentHashMap<>(); // safe

A HashMap written by two threads at once can lose entries or corrupt its internal structure. It won't throw — it'll just be wrong later, somewhere else, which is the worst kind of bug to trace.

The ThreadLocal trap, and why it matters for multi-tenancy

Sometimes you genuinely need per-request state that's awkward to thread through every method signature. In Raaqib, every query has to be scoped to the current tenant, and the tenant is resolved once per request from the JWT. Passing it into every repository call by hand is exactly the kind of thing a developer eventually forgets to do.

So it lives in a ThreadLocal — a variable where each thread gets its own independent copy:

public final class TenantContext {

    private static final ThreadLocal<UUID> CURRENT = new ThreadLocal<>();

    public static void set(UUID tenantId) { CURRENT.set(tenantId); }

    public static UUID require() {
        UUID id = CURRENT.get();
        if (id == null) throw new IllegalStateException("No tenant bound");
        return id;
    }

    public static void clear() { CURRENT.remove(); }
}

Now here's the part that turns a convenience into an incident. Request threads are pooled. When a request finishes, its thread goes back in the pool and serves somebody else — and it still carries whatever you left in the ThreadLocal.

Forget to clear it, and the next request on that thread inherits the previous tenant's id. If that request belongs to a different store, you have just served one customer another customer's stock. Silently. Only under load, only sometimes.

So the clear is not optional, and it goes in a finally:

try {
    TenantContext.set(tenantFromJwt);
    chain.doFilter(request, response);
} finally {
    TenantContext.clear(); // always, even if the request blew up
}

Two details worth defending. clear() calls remove() rather than set(null) — set(null) leaves an entry in the thread's map, which keeps a reference alive and is how ThreadLocal memory leaks happen in long-lived pools. And require() throws instead of returning null, because an unbound tenant means a query is about to run unfiltered, and I would much rather fail the request than return the wrong store's data.

The general rule: a ThreadLocal on a pooled thread is only as safe as its cleanup.

When you actually want state, don't put it in a singleton

If a bean genuinely needs per-request data, say so in the scope instead of improvising:

@Component
@Scope(value = WebApplicationContext.SCOPE_REQUEST,
       proxyMode = ScopedProxyMode.TARGET_CLASS)
public class CheckoutBasket { ... }

Spring creates one per request and throws it away afterwards. The proxyMode is what lets you inject it into a singleton — you get a proxy that resolves to the right instance per request.

That said, I use this rarely. A stateless service that takes what it needs as parameters is simpler than any scoping strategy, and it's trivially testable.

The summary I'd give in an interview

Spring beans are singletons, so every mutable field on a @Service is shared by every request thread — that's why SimpleDateFormat as a field corrupts data and DateTimeFormatter doesn't. Local variables are always safe because each thread has its own stack. volatile gives visibility but not atomicity, so count++ is still a race; use AtomicInteger or synchronized depending on whether you need one field or several to move together. And ThreadLocal is the right tool for per-request context like a tenant id, as long as you clear it with remove() in a finally block — otherwise a pooled thread hands the previous request's tenant to the next one.