When Can Flawed Code Stay Unchanged? A Deep Dive into Concurrency and Locking

What code?

During a recent code review the team spotted a snippet that updates a configuration record:

Config cfg = configMapper.selectById(id);
cfg.setKey(newKey);
cfg.setValue(newValue);
configMapper.updateById(cfg);

The immediate question was whether this code has a concurrency problem.

Why it can be problematic

If two requests modify the same row concurrently—one changing key / value and another changing an unrelated account field—whichever update runs last will overwrite the other change. The example shows that after selecting the record, another request may modify account before the original update is persisted, causing the account change to be lost.

In the team’s specific context the configuration is edited by a single person at a very low frequency, so they concluded the risk is negligible and left the code unchanged.

Alternative approaches

To avoid the race condition, the code can be rewritten to update only the needed fields:

Config cfg = configMapper.selectById(id);
Config cfgForUpdate = new Config();
cfgForUpdate.setId(id);
cfgForUpdate.setKey(newKey);
cfgForUpdate.setValue(newValue);
configMapper.updateById(cfgForUpdate);

Or use optimistic locking by adding a last_update_time (or version) column and updating with a condition that the timestamp matches the one read:

UPDATE config SET key = ?, value = ?
WHERE id = ? AND last_update_time = :lastUpdateTime;

If the timestamps differ, the update fails and the front‑end can prompt the user to retry.

Think again

Even with the selective‑update version, a similar race can occur if a user leaves a page, another user updates the record, and the first user later submits stale data. The same locking strategy (optimistic lock) resolves this.

When you must not ignore the issue

For financial‑sensitive operations such as orders, loans, or accounting, any lost update is unacceptable. In these cases the code must use a strict pessimistic lock (e.g., database row lock) combined with validation steps before updating, and must always release the lock afterward.

The pattern can be summarized as “one lock, two checks, three updates, four releases”.

Double‑checked locking example

public class Singleton {
    private static volatile Singleton instance;
    private Singleton() {}
    public static Singleton getInstance() {
        if (instance == null) {
            synchronized (Singleton.class) {
                if (instance == null) {
                    instance = new Singleton();
                }
            }
        }
        return instance;
    }
}

Although the example omits an explicit “release” step, the synchronized block implicitly releases the lock.

Other reflections

Code review often surfaces low‑probability edge cases that are expensive to fix. Teams must weigh the cost of fixing against the risk, using metrics like cost‑benefit analysis, and decide whether to monitor and alert instead of changing the code.