CWE-362: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') - Java
Overview
Shared mutable state accessed from multiple threads - a field on a Spring singleton bean, a static counter, an in-memory cache - is a common source of this weakness in Java, since neither the JVM nor common frameworks synchronize access automatically. The primary fix is the synchronized keyword or java.util.concurrent.locks.ReentrantLock around the full read-modify-write sequence, or the java.util.concurrent.atomic classes (AtomicInteger, AtomicLong, AtomicReference) for single-variable updates. For state backed by a relational database - the more common case in a web application, where the "shared resource" is a row any server instance can touch - prefer SELECT ... FOR UPDATE inside a transaction or JPA/Hibernate @Version optimistic locking over an application-level lock, since a JVM-level lock only protects the process that holds it.
Common Vulnerable Patterns
Unsynchronized Singleton Bean State
// VULNERABLE - a Spring singleton bean's field is mutated with no synchronization
@Service
public class RequestCounterService {
private int count;
public void increment() {
count = count + 1; // read-modify-write: not atomic
}
public int getCount() {
return count;
}
}
// Attack: fire 1,000 concurrent requests, each calling increment() once.
// Result: getCount() often returns less than 1,000 - some increments are lost.
Why this is vulnerable: Spring's default bean scope is singleton, and the container invokes it concurrently from every in-flight request thread. count = count + 1 is a read, an add, and a write as separate bytecode steps; if two threads interleave between the read and the write, both compute the same new value from the same stale read and one increment is lost.
Check-Then-Act on a Shared Balance Field
// VULNERABLE - check and update are two separate, unsynchronized steps
public class Account {
private int balance;
public Account(int initialBalance) {
this.balance = initialBalance;
}
public void withdraw(int amount) {
if (balance < amount) {
throw new IllegalStateException("insufficient funds");
}
// RACE WINDOW: another thread can withdraw here before this line runs
balance -= amount;
}
}
// Attack: two threads call withdraw(100) concurrently on an Account with
// balance = 100. Both read balance = 100 and both pass the check before
// either writes. Result: balance ends at -100 instead of one call failing.
Why this is vulnerable: Nothing prevents two threads from being inside withdraw at the same time, both having read the same starting balance before either one writes the decremented value back.
Separate SELECT Then UPDATE (JPA, No Version Column)
// VULNERABLE - no @Version, the second save() silently overwrites the first
@Transactional
public void withdraw(Long accountId, BigDecimal amount) {
Account account = accountRepository.findById(accountId)
.orElseThrow(() -> new AccountNotFoundException());
if (account.getBalance().compareTo(amount) < 0) {
throw new InsufficientFundsException();
}
account.setBalance(account.getBalance().subtract(amount));
accountRepository.save(account); // lost update if another transaction wrote first
}
// Attack: two concurrent withdraw(accountId, 100) transactions when the
// balance is 100. Both load balance = 100, both pass the check, both save
// balance = 0. One withdrawal is completely lost.
Why this is vulnerable: Without a @Version column, Hibernate's flush issues a plain UPDATE ... WHERE id = ? with no check that the row is unchanged since it was read. Whichever transaction commits second silently replaces the first transaction's write.
Secure Patterns
synchronized for the Full Critical Section
// SECURE - synchronized protects the full read-modify-write sequence
public class Account {
private int balance;
public Account(int initialBalance) {
this.balance = initialBalance;
}
public synchronized void withdraw(int amount) {
if (balance < amount) {
throw new IllegalStateException("insufficient funds");
}
balance -= amount;
}
// synchronized on the read as well: an unsynchronized getter can return a
// stale value even though every write went through the lock
public synchronized int getBalance() {
return balance;
}
}
Why this works: synchronized on the instance method acquires the object's intrinsic lock for the whole method body, so only one thread can execute withdraw on a given Account instance at a time. The balance check and the decrement always run as one atomic unit relative to other callers, and the lock is released automatically even if the method throws.
getBalance() is synchronized for a different reason from withdraw(). There is no check-then-act in a getter, but without the lock there is no happens-before edge to the writes, so a reader thread may observe a value from before another thread's withdraw - indefinitely, since the JIT is free to hoist the field read out of a loop. synchronized on the getter, volatile on the field, or an AtomicInteger all supply that edge. Leaving the getter unguarded is the usual way a correctly locked class still reports the wrong number, and it is also what makes an assertion in a test unreliable.
ReentrantLock for More Control
import java.util.concurrent.locks.ReentrantLock;
// SECURE - ReentrantLock offers timed/interruptible acquisition that
// synchronized does not
public class Account {
private final ReentrantLock lock = new ReentrantLock();
private int balance;
public Account(int initialBalance) {
this.balance = initialBalance;
}
public void withdraw(int amount) {
lock.lock();
try {
if (balance < amount) {
throw new IllegalStateException("insufficient funds");
}
balance -= amount;
} finally {
lock.unlock();
}
}
}
Why this works: ReentrantLock provides the same mutual exclusion as synchronized, plus tryLock(timeout) for bounded waiting and lockInterruptibly() for cancellation - useful when a critical section must not block indefinitely. The try/finally is mandatory here (unlike synchronized, which releases automatically): unlock() in finally guarantees the lock is released on every exit path, including exceptions.
AtomicInteger for a Simple Counter
import java.util.concurrent.atomic.AtomicInteger;
// SECURE - AtomicInteger avoids the need for a lock on a single counter
@Service
public class RequestCounterService {
private final AtomicInteger count = new AtomicInteger(0);
public void increment() {
count.incrementAndGet();
}
public int getCount() {
return count.get();
}
}
Why this works: incrementAndGet() is backed by a single compare-and-swap hardware instruction - there is no window in which two threads can both read the same value before either writes it back. This avoids the cost and contention of a full lock for the common case of a single counter or flag.
ConcurrentHashMap.compute for Shared Collections
import java.util.concurrent.ConcurrentHashMap;
// SECURE - compute() makes the check-then-act on a map value atomic
public class InventoryService {
private final ConcurrentHashMap<String, Integer> stock = new ConcurrentHashMap<>();
public boolean tryReserve(String sku, int quantity) {
boolean[] reserved = {false};
stock.compute(sku, (key, current) -> {
int available = current == null ? 0 : current;
if (available < quantity) {
reserved[0] = false;
return available;
}
reserved[0] = true;
return available - quantity;
});
return reserved[0];
}
}
Why this works: ConcurrentHashMap guarantees thread-safe individual operations, but a separate get() followed by put() is still a check-then-act race even on a concurrent map. compute() runs its remapping function atomically per key - only one thread's function executes for a given key at a time - so the entire "check quantity, then decrement" decision happens inside one atomic call.
JPA Optimistic Locking with @Version
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Version;
import java.math.BigDecimal;
@Entity
public class Account {
@Id
private Long id;
private BigDecimal balance;
@Version // SECURE - Hibernate includes this column in the WHERE clause
// and increments it on every UPDATE
private Long version;
// getters/setters omitted
}
// SECURE - the UPDATE fails if the version changed since the row was read.
// The conflict is NOT caught here: it has to leave the transaction.
@Service
public class AccountService {
private final AccountRepository accountRepository;
public AccountService(AccountRepository accountRepository) {
this.accountRepository = accountRepository;
}
@Transactional
public void withdraw(Long accountId, BigDecimal amount) {
Account account = accountRepository.findById(accountId)
.orElseThrow(AccountNotFoundException::new);
if (account.getBalance().compareTo(amount) < 0) {
throw new InsufficientFundsException();
}
account.setBalance(account.getBalance().subtract(amount));
accountRepository.saveAndFlush(account);
}
}
// A separate bean, so the call goes through the transactional proxy and the
// catch runs after the transaction has already committed or rolled back
@Service
public class WithdrawalFacade {
private final AccountService accounts;
public WithdrawalFacade(AccountService accounts) {
this.accounts = accounts;
}
public boolean tryWithdraw(Long accountId, BigDecimal amount) {
try {
accounts.withdraw(accountId, amount);
return true;
} catch (OptimisticLockingFailureException e) {
// org.springframework.dao.OptimisticLockingFailureException - Spring
// Data translates Hibernate's StaleObjectStateException into this
// hierarchy. Retry against fresh data, or report the conflict.
return false;
}
}
}
Why this works: @Version marks the column as a concurrency token; Hibernate generates an UPDATE ... WHERE id = ? AND version = ? and increments version on every successful write. If a concurrent transaction already updated the row, the WHERE clause matches zero rows and Hibernate raises StaleObjectStateException instead of silently applying a stale write.
Two things about that exception decide whether the handling works, and both were verified on Spring Boot 3.5.5 with Hibernate 6 and JDK 26.
saveAndFlush does not throw jakarta.persistence.OptimisticLockException. Spring Data repositories are proxied with exception translation, so what reaches the caller is org.springframework.orm.ObjectOptimisticLockingFailureException, wrapping the Hibernate exception. Measured: instanceof jakarta.persistence.OptimisticLockException is false, instanceof org.springframework.dao.OptimisticLockingFailureException is true. A catch (OptimisticLockException e) on the JPA type therefore never fires with Spring Data, and neither does @Retryable(retryFor = OptimisticLockException.class) - the conflict propagates uncaught through a handler written to absorb it. Catch Spring's OptimisticLockingFailureException (or its ObjectOptimisticLockingFailureException subclass) when the repository is a Spring Data one; the JPA type is what a bare EntityManager.flush() throws.
Catching it inside @Transactional and returning normally does not work either. Any exception that escapes a repository call marks the transaction rollback-only, so swallowing it and returning false gets the caller org.springframework.transaction.UnexpectedRollbackException: Transaction silently rolled back because it has been marked as rollback-only at commit - measured, with the correct catch clause in place. The conflict has to cross the transaction boundary before anything decides what to do about it.
That is why tryWithdraw lives on a different bean. A try/catch around a this.withdraw(...) call in the same class would not help: a self-invocation never passes through the Spring proxy, so @Transactional would not apply to it at all and the whole optimistic-locking mechanism would run outside a transaction. The same reasoning is why @Retryable works when it is applied alongside @Transactional - its advice wraps the transactional proxy rather than running inside it, so each retry gets a fresh transaction.
SELECT ... FOR UPDATE for Pessimistic Locking
import jakarta.persistence.LockModeType;
import org.springframework.data.jpa.repository.Lock;
import org.springframework.data.jpa.repository.Query;
public interface AccountRepository extends JpaRepository<Account, Long> {
// SECURE - locks the row for the duration of the transaction, forcing
// concurrent transactions on the same row to wait rather than race
@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("SELECT a FROM Account a WHERE a.id = :id")
Optional<Account> findByIdForUpdate(@Param("id") Long id);
}
@Transactional
public void withdraw(Long accountId, BigDecimal amount) {
Account account = accountRepository.findByIdForUpdate(accountId)
.orElseThrow(AccountNotFoundException::new);
if (account.getBalance().compareTo(amount) < 0) {
throw new InsufficientFundsException();
}
account.setBalance(account.getBalance().subtract(amount));
// save happens automatically on transaction commit for a managed entity
}
Why this works: PESSIMISTIC_WRITE issues a SELECT ... FOR UPDATE and holds the row lock until the transaction commits or rolls back. Any other transaction attempting to read the same row with a write lock blocks until this one finishes, so the balance this code checked cannot change out from under it before the update is flushed. Unlike optimistic locking, this blocks rather than fails a conflicting concurrent writer, which is preferable when conflicts are expected to be frequent and a retry loop would thrash.
Framework-Specific Guidance
Spring Boot
// SECURE - do not assume a singleton @Service or @Component is race-free
// because "only one instance handles requests" - the container invokes it
// from every concurrent request thread
@Service
public class WalletService {
private final AccountRepository accountRepository;
public WalletService(AccountRepository accountRepository) {
this.accountRepository = accountRepository;
}
// The Spring DAO type, not jakarta.persistence.OptimisticLockException -
// Spring Data translates Hibernate's exception into this hierarchy, and
// naming the JPA type here means the method is never retried at all.
@Retryable(retryFor = OptimisticLockingFailureException.class,
maxAttempts = 3, backoff = @Backoff(delay = 50, multiplier = 2))
@Transactional
public void withdraw(Long accountId, BigDecimal amount) {
// see the @Version pattern above; @Retryable (Spring Retry) retries
// the whole method - including a fresh transaction and a fresh read -
// on a detected conflict
}
}
Why this works: Spring's default singleton scope means one bean instance serves every request thread concurrently, so any mutable field on the bean itself is shared state (see the vulnerable counter example above) - fields should be avoided or synchronized, and business state belongs in the database, protected by @Version or PESSIMISTIC_WRITE. Spring Retry's @Retryable gives a losing transaction another attempt against fresh data automatically, instead of requiring hand-written retry loops at every call site.
The retry advice runs outside the transaction advice, which is what makes it work: each attempt gets a new transaction, so the second attempt re-reads the row and its current version rather than retrying against the stale entity that lost. Verified on Spring Boot 3.5.5 with @EnableRetry: a conflict on attempt 1 was followed by attempt 2 opening a fresh transaction and committing. Naming the JPA exception type instead produced no retry at all - the method failed after one attempt with ObjectOptimisticLockingFailureException, which the annotation was not listening for. A bare @Retryable with no backoff retries immediately; @Backoff is what stops a contended row turning into a livelock.
Hibernate
// SECURE - explicit isolation level when the default READ_COMMITTED is not
// strict enough for a particular operation.
// SERIALIZABLE can make this method FAIL rather than wait, so the caller
// needs a retry - see @Retryable above.
@Transactional(isolation = Isolation.SERIALIZABLE)
public void transferFunds(Long fromId, Long toId, BigDecimal amount) {
// the two reads and the two writes are serialized against every other
// transaction touching the same rows
}
Why this works: The database's default isolation level (commonly READ_COMMITTED) still permits lost updates on a plain read-then-write outside of @Version or explicit row locking. Raising the isolation level to SERIALIZABLE for a specific high-value operation trades throughput for the strongest consistency guarantee the database offers; reserve it for operations where the cost is justified rather than applying it globally.
How a conflict presents under SERIALIZABLE is engine-specific, and it is not always "wait". Engines implement the guarantee in two different ways: by taking locks so a conflicting transaction blocks, or by letting transactions run optimistically and aborting one at commit with a serialization failure. PostgreSQL's SSI does the latter and returns SQLSTATE 40001, which Spring surfaces as a ConcurrencyFailureException. A method annotated SERIALIZABLE with no retry around it is therefore a correctness fix on one engine and an intermittent 500 on another - and the failure only appears under real concurrency, so it usually reaches production. Check which behaviour your engine has, and wrap the method in the same bounded, backed-off retry the optimistic-locking section uses; the two are the same problem with a different trigger.
Considerations
A lock is only as wide as the thing holding it. An in-process lock serialises the threads inside one instance and does nothing about a second instance, so a fix that works on a developer machine can fail the moment the service is scaled out or restarted behind a load balancer. Decide first where the shared state actually lives. If it is a database row, the serialisation has to happen in the database - a row lock, a conditional update, or a version column. If it is genuinely in-process and stays that way, an in-process lock is correct and cheaper.
Not every race is worth fixing. Two requests overwriting a display preference, or a page-view counter losing an increment, is a race with no security consequence and often no user-visible one. The ones that matter change a decision: a balance check, a quota, a one-time token being redeemed, a permission being evaluated. Fixing a benign race costs throughput and adds a failure mode, so say which category the finding is in before reaching for a lock.
synchronized is per-JVM, and that is the usual gap. It serialises threads
within one instance and means nothing across a cluster, so a check-then-act on
a database row needs the database to arbitrate. Note also that locking on a
String literal or a boxed primitive silently shares a monitor with unrelated
code, because those instances are interned.
Optimistic and pessimistic locking fail in opposite directions. A pessimistic lock makes every caller wait, so it is predictable but caps throughput and can deadlock if two paths take locks in different orders. Optimistic concurrency lets callers proceed and rejects the loser, which is faster when conflicts are rare and degenerates into wasted work and retries when they are common. Pick by how often the same row is genuinely contended, not by which is easier to write.
Retries need a bound and a backoff. A conflict-and-retry loop with neither turns a contended row into a livelock under load - every caller retrying immediately, none making progress. Cap the attempts, back off between them, and decide what the caller sees when the cap is reached. "Try again" is a legitimate answer; silently returning stale data is not.
Testing
A re-scan cannot confirm this fix. The tool sees synchronized where there was
none and reports the finding closed; it cannot tell whether the critical
section covers the whole decision, whether every path takes the same monitor,
or whether the locked version still serves a legitimate request.
Starting threads together is not enough. A CountDownLatch releasing five
threads at once looks like it forces contention, and it does not force the
interleaving that matters: the window between the check and the write is a
handful of nanoseconds, and each thread usually gets through the whole method
before the next one is scheduled. Measured on JDK 26, a latch-only version of
the test below returned the "correct" answer of 3 against an unsynchronized
Account on 199 runs out of 200 - a false pass 99.5% of the time, on a
16-core machine that is friendlier to this race than any CI box.
Pin the interleaving instead. Give the class under test a hook between the
check and the write and make the hook a CyclicBarrier: every thread that
passed the check waits until all of them have, so the stale read is guaranteed.
Give the barrier a timeout so a correctly synchronized implementation - where
only one thread is ever inside the method - waits it out and completes rather
than deadlocking.
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicInteger;
import static org.junit.jupiter.api.Assertions.*;
// Production code passes no hook; only the test does.
public class Account {
private final Runnable betweenCheckAndWrite;
private int balance;
public Account(int initialBalance) { this(initialBalance, () -> {}); }
public Account(int initialBalance, Runnable betweenCheckAndWrite) {
this.balance = initialBalance;
this.betweenCheckAndWrite = betweenCheckAndWrite;
}
public synchronized void withdraw(int amount) {
if (balance < amount) {
throw new IllegalStateException("insufficient funds");
}
betweenCheckAndWrite.run();
balance -= amount;
}
public synchronized int getBalance() { return balance; }
}
@Test
void concurrentWithdrawals_neverOverdraftTheAccount() throws InterruptedException {
int threadCount = 5;
CyclicBarrier barrier = new CyclicBarrier(threadCount);
Account account = new Account(100, () -> {
try {
// Every thread past the balance check stops here until all five
// are. The timeout is what lets the synchronized version through:
// only one thread ever arrives, so it waits 200 ms and continues.
barrier.await(200, TimeUnit.MILLISECONDS);
} catch (Exception expected) {
// TimeoutException here means the lock is doing its job
}
});
ExecutorService pool = Executors.newFixedThreadPool(threadCount);
AtomicInteger succeeded = new AtomicInteger();
for (int i = 0; i < threadCount; i++) {
pool.submit(() -> {
try {
account.withdraw(30);
succeeded.incrementAndGet();
} catch (IllegalStateException expected) {
// the calls that correctly lose the race
}
});
}
pool.shutdown();
assertTrue(pool.awaitTermination(10, TimeUnit.SECONDS));
// Exactly 3 of the 5 concurrent withdrawals of 30 succeed against 100,
// and the balance lands on the arithmetic those 3 imply.
assertEquals(3, succeeded.get());
assertEquals(10, account.getBalance());
}
Measured on JDK 26 over 50 runs, this returns 5 successes every time against the unsynchronized version - so it fails, loudly and repeatably, exactly when it should.
Assert these, with the result each should produce:
- Accept, single call.
withdraw(30)on a balance of 100 returns andgetBalance()is exactly 70. A lock that refuses every caller passes the concurrency assertion above and fails this one. - Accept, at the boundary.
withdraw(100)on 100 succeeds and leaves 0. An off-by-one in the guard shows up here and nowhere else. - Reject, past the boundary.
withdraw(1)on 0 throwsIllegalStateExceptionand leaves the balance at 0 - unchanged, not merely non-negative. - Concurrent. The barrier test above: 3 successes and
getBalance() == 10. Assert the balance as well as the count; a lost update can produce the right number of successes and the wrong total. - The monitor is released on the rejection path. After a refused withdrawal, a later legitimate call on the same instance still returns.
synchronizedgives this for free; aReentrantLockwhoseunlock()is not in afinallydeadlocks here and passes everything above. - Every path takes the same lock. Where a second method touches
balance, drive both concurrently and assert the invariant. A class that synchronizeswithdrawand leavesdepositunguarded passes every assertion in this list.
For state in a database, none of the above reaches the real weakness - a JVM monitor is invisible to a second instance. Drive the endpoint from several processes and assert on the persisted row.
Common Pitfalls
- Locking on a different object than the one guarding the resource: Synchronizing on
thisin one method and on a separate lock field in another, or synchronizing on aStringliteral (which the JVM may intern and share with unrelated code), lets two call paths run concurrently even though both "look" synchronized. - Using
ConcurrentHashMapbut still doingget()thenput(): The map's individual operations are thread-safe, but that pair of calls is still a check-then-act race - another thread can modify the value between the two calls. Usecompute()/computeIfAbsent()/merge()so the whole decision happens inside one atomic call. - Catching
OptimisticLockExceptionand discarding it: Swallowing the exception without reloading and retrying makes the@Versioncolumn pointless - the conflict is detected but the losing write's data is dropped with no indication to the caller. - Catching the JPA exception type when the repository is a Spring Data one:
saveAndFlushthrowsorg.springframework.orm.ObjectOptimisticLockingFailureException, which is not ajakarta.persistence.OptimisticLockException- so acatchor a@Retryable(retryFor = ...)naming the JPA type never fires, and the conflict propagates through the very code written to handle it. Catchorg.springframework.dao.OptimisticLockingFailureException. - Handling the conflict inside the
@Transactionalmethod: an exception escaping a repository call marks the transaction rollback-only, so catching it and returning normally gets the callerUnexpectedRollbackExceptionat commit instead of the value you returned. Let the conflict cross the transaction boundary and decide what it means outside it. - Assuming a singleton Spring bean without mutable fields is automatically safe, then adding a cache field later: A stateless bean is race-free by construction, but adding an instance-level cache, counter, or buffer to what was a stateless service silently reintroduces this weakness unless the new field is synchronized or made atomic from the start.
Dependencies and Installation
No additional dependency is required for synchronized, ReentrantLock, java.util.concurrent.atomic, or ConcurrentHashMap - all are part of the JDK. @Version and @Lock(LockModeType.PESSIMISTIC_WRITE) require a JPA provider such as Hibernate (already a dependency for any JPA-based project); @Retryable requires the spring-retry artifact plus spring-boot-starter-aop.