Synchronising on a boxed Integer or an interned String

What this error actually means

A monitor belongs to an *object identity*, not to a variable or a value. Two references that point at the same object share one lock, whether or not the code that wrote them knows the other exists. Boxed primitives and interned strings are the two places where the JVM deliberately hands out shared identities, which turns a lock that reads as private into one that is effectively global.

`Integer.valueOf` is required to cache and return the same instance for values between −128 and 127, and autoboxing goes through `valueOf`. So `synchronized (userId)` where `userId` is an `Integer` of 42 locks the *same object* as every other piece of code in the JVM — including library code — that boxes 42. `Boolean.valueOf` is worse: there are exactly two instances in existence, so locking on a `Boolean` serialises everything in the process that does the same.

String literals are interned into a pool shared by the whole JVM, and `String.intern()` puts runtime-computed strings there too. `synchronized ("acme")` in your billing code and `synchronized (tenantName.intern())` in a reporting job acquire one monitor. Unlike the boxed case there is no small range limiting the damage — any two literals with the same characters collide, across modules, across libraries, across code you did not write.

The failures this produces are the hardest kind to attribute, because the deadlock cycle or the contention runs between classes that have no relationship in the source. You read both sides, neither has any business blocking the other, and the dump tells you they are fighting over a `java.lang.Integer`. The second, quieter failure mode is the mirror image: locking on a boxed value *inside* the cache range works while values are small and silently stops providing mutual exclusion at 128, when `valueOf` starts returning fresh instances and every thread locks its own object.

How to diagnose it

jcmd <pid> Thread.print | grep -E "locked <0x|waiting to lock" | grep -E "java.lang.(String|Integer|Long|Boolean|Short|Byte|Character)"

grep -rn -E "synchronized *\( *(\"|[a-zA-Z_]+\.intern\(\))" src/main/java
grep -rn -E "synchronized *\( *[a-zA-Z_]*([Ii]d|[Cc]ount|[Ff]lag) *\)" src/main/java

System.out.println(Integer.valueOf(42) == Integer.valueOf(42));   // true
System.out.println(Integer.valueOf(200) == Integer.valueOf(200)); // false
System.out.println("acme" == "ac" + "me");                        // true (compile-time constant)

./gradlew test --tests '*ConcurrencyTest*' -Dtest.accountId=200

grep -rn -B 2 "synchronized (" src/main/java | grep -E "private .* (Object|Lock) " | grep -v final

The fix

Lock on a dedicated, private, final object that exists for no other purpose: `private final Object lock = new Object();`. It has one identity, nobody else can obtain it, and it cannot be reassigned. Every other choice of lock target is an argument you have to win; this one needs no argument.

For per-key locking, get the lock from a map keyed by the key rather than locking the key itself: `locks.computeIfAbsent(accountId, k -> new Object())`. The `computeIfAbsent` guarantees one lock object per key, and the identity is yours rather than shared with the JVM. Bound the map or use weak values, otherwise the lock map becomes an unbounded cache keyed by everything you have ever seen.

Better for most per-key cases: use lock striping and skip the per-key map entirely. A fixed array of `N` locks indexed by `Math.floorMod(key.hashCode(), N)` gives bounded memory, no lifecycle problem, and contention only between keys that collide. Guava’s `Striped` provides this directly if you would rather not hand-roll it.

Where the critical section is a single read-modify-write on a map entry, delete the lock and use `ConcurrentHashMap.compute` or `merge`. Per-key atomicity without any lock object is strictly better than per-key locking done carefully, because there is nothing to get wrong.

Never synchronise on `this` in a class whose instances are handed out. Use a private lock so your mutual exclusion is not part of your public surface, and so a framework proxying or caching your object cannot interfere with it.

Make every lock field `final` and declare it next to the state it guards, ideally with a comment naming that state. Reassignment bugs and "which lock protects this field" confusion both disappear when the pairing is local and immutable.

// Shares a monitor with any code in the JVM that boxes the same value,
// AND stops excluding anything at all once ids exceed 127.
void credit(Integer accountId, BigDecimal amt) {
    synchronized (accountId) { ... }
}

// Shares a monitor with every user of the literal "acme" in the process
synchronized (tenantName.intern()) { ... }

// Correct: a private identity that nothing else can reach
private final Object lock = new Object();
void credit(long accountId, BigDecimal amt) {
    synchronized (lock) { ... }
}

// Correct per-key locking: the identity comes from a map you own
private final ConcurrentHashMap<Long, Object> locks = new ConcurrentHashMap<>();
void credit(long accountId, BigDecimal amt) {
    Object keyLock = locks.computeIfAbsent(accountId, k -> new Object());
    synchronized (keyLock) { ... }
}

// Usually better: bounded striping, no lock lifecycle to manage
private final Object[] stripes = IntStream.range(0, 64)
        .mapToObj(i -> new Object()).toArray();
void credit(long accountId, BigDecimal amt) {
    synchronized (stripes[(int) Math.floorMod(accountId, stripes.length)]) { ... }
}

// Best where the section is one read-modify-write: no lock at all
balances.merge(accountId, amt, BigDecimal::add);

How to stop it coming back

• Ban `synchronized` on boxed types, `String`, and `Boolean` with a static-analysis rule — SpotBugs flags these directly
• Declare every lock as `private final Object` beside the state it guards, and name the guarded state in a one-line comment
• Never synchronise on `this` or on any object that leaves the class
• Run per-key concurrency tests with key values on both sides of 128 so autobox-cache behaviour cannot hide a broken lock
• Prefer striping or atomic map operations over per-key lock maps; a lock map with no bound is an unbounded cache
• When reading a deadlock dump, check the monitor types first — a JDK value type as a monitor is an immediate finding

FAQ

Related

Other errors engineers hit next to this one

• SettingWithCopyWarning: A value is trying to be set on a copy of a slice
• celery.exceptions.WorkerLostError: Worker exited prematurely
• requests.exceptions.ReadTimeout: HTTPSConnectionPool read timed out
• UnicodeDecodeError: 'utf-8' codec can't decode byte
• AssertionError: daemonic processes are not allowed to have children
• [CRITICAL] WORKER TIMEOUT (pid:1234)
• Mutable default argument retains state across calls
• CommitFailedException: Commit cannot be completed since the group has already rebalanced