Empty Synchronized Block

Description

Empty Synchronized Block is a code quality weakness in Java where a synchronized block contains no executable statements. Synchronization is used to ensure that only one thread can access a critical section at a time, protecting shared resources from concurrent modification. An empty synchronized block acquires a lock, does nothing, and releases it—providing no actual protection for any shared data. This pattern typically occurs when developers comment out code within a synchronized block without removing the synchronization itself, or when code is refactored incorrectly, leaving vestigial synchronization logic.

Risk

Empty synchronized blocks create multiple problems. While the code appears to perform synchronization, it actually provides no thread safety for any subsequent operations. Developers may falsely believe their code is thread-safe when it is not. The empty block still incurs synchronization overhead, causing unnecessary performance degradation. The pattern indicates potential bugs—either critical code was accidentally removed, or the synchronization itself should have been removed. In security contexts, race conditions that the synchronization was meant to prevent may still exist, leading to data corruption or inconsistent state.

Solution

Investigate the intent behind empty synchronized blocks. If the synchronized code was removed but synchronization is still needed, add the appropriate thread-safe operations. If the protected code is no longer needed, remove the entire synchronized block to eliminate unnecessary lock acquisition. Use code review and static analysis tools to detect empty synchronized blocks. When refactoring code, ensure that synchronized blocks and their contents are handled together. Document the purpose of synchronization to prevent accidental removal of critical code.

Common Consequences

Impact	Details
Other	Scope: Other Quality Degradation - The code suggests synchronization but provides none, indicating bugs or code rot.
Integrity	Scope: Integrity Modify Application Data - Race conditions that should have been prevented may still occur, corrupting shared data.

Example Code

Vulnerable Code

// Vulnerable: Empty synchronized block
public class VulnerableEmptySync {
    private int sharedCounter = 0;

    public void incrementCounter() {
        synchronized(this) {
            // Empty - synchronization does nothing!
        }

        // This code runs OUTSIDE the synchronized block
        // and is NOT thread-safe!
        sharedCounter++;
    }
}

// Vulnerable: Code removed, sync block left behind
public class VulnerableCommentedOut {
    private List<String> items = new ArrayList<>();

    public void processItem(String item) {
        synchronized(items) {
            // Code was here but got commented out during refactoring:
            // items.add(item);
            // processInternal(item);
        }

        // Developer thought this was synchronized, but it's not!
        items.add(item);  // Race condition!
    }
}

// Vulnerable: Misplaced synchronization
public class VulnerableMisplaced {
    private volatile boolean flag = false;
    private String data = null;

    public void setData(String value) {
        // Vulnerable: Empty sync block before critical section
        synchronized(this) {
            // Oops, forgot to put the assignment here
        }

        // Assignment is NOT synchronized
        data = value;
        flag = true;
    }

    public String getData() {
        synchronized(this) {
            // Empty block
        }

        // Read is NOT synchronized - may see inconsistent state
        if (flag) {
            return data;
        }
        return null;
    }
}

// Vulnerable: Result of incomplete refactoring
public class VulnerableRefactored {
    private Map<String, Object> cache = new HashMap<>();

    // Original code had cache operations inside synchronized block
    // After moving to ConcurrentHashMap, dev forgot to remove sync block
    public Object getFromCache(String key) {
        synchronized(cache) {
            // Was: return cache.get(key);
            // Now empty after switching to ConcurrentHashMap
        }

        // New code using ConcurrentHashMap
        // The synchronized block above is useless
        return cache.get(key);
    }
}

// Vulnerable: Synchronization on wrong object
public class VulnerableWrongLock {
    private final Object lock = new Object();
    private int value;

    public void setValue(int v) {
        synchronized(lock) {
            // Empty block - intended to protect value assignment below
        }
        // Not protected!
        value = v;
    }

    public int getValue() {
        synchronized(new Object()) {
            // Synchronizing on new object each time - useless!
        }
        return value;
    }
}

Fixed Code

// Fixed: Synchronized block with actual content
public class SecureEmptySync {
    private int sharedCounter = 0;

    public void incrementCounter() {
        synchronized(this) {
            // Fixed: Critical operation is inside the synchronized block
            sharedCounter++;
        }
    }

    public int getCounter() {
        synchronized(this) {
            return sharedCounter;
        }
    }
}

// Fixed: Complete removal of unnecessary sync or proper inclusion
public class SecureProperSync {
    private List<String> items = new ArrayList<>();

    // Option 1: Remove sync block if not needed
    public void processItemNoSync(String item) {
        // If thread safety is handled elsewhere, remove sync entirely
        items.add(item);
    }

    // Option 2: Include code in sync block
    public void processItemSynced(String item) {
        synchronized(items) {
            // Fixed: Code inside synchronized block
            items.add(item);
            processInternal(item);
        }
    }

    // Option 3: Use concurrent collection instead
    private List<String> concurrentItems =
        Collections.synchronizedList(new ArrayList<>());

    public void processItemConcurrent(String item) {
        // No sync block needed for add
        concurrentItems.add(item);
    }
}

// Fixed: Proper synchronization placement
public class SecureProperPlacement {
    private volatile boolean ready = false;
    private String data = null;
    private final Object lock = new Object();

    public void setData(String value) {
        synchronized(lock) {
            // Fixed: All related operations inside sync block
            data = value;
            ready = true;
        }
    }

    public String getData() {
        synchronized(lock) {
            // Fixed: Read operation inside sync block
            if (ready) {
                return data;
            }
            return null;
        }
    }
}

// Fixed: Complete refactoring - remove vestigial synchronization
public class SecureRefactored {
    // Fixed: Using ConcurrentHashMap - no sync blocks needed
    private Map<String, Object> cache = new ConcurrentHashMap<>();

    public Object getFromCache(String key) {
        // No synchronized block - ConcurrentHashMap handles it
        return cache.get(key);
    }

    public void putInCache(String key, Object value) {
        cache.put(key, value);
    }

    // If atomic operations are needed:
    public Object computeIfAbsent(String key) {
        return cache.computeIfAbsent(key, k -> createValue(k));
    }
}

// Fixed: Use higher-level concurrency utilities
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class SecureReadWriteLock {
    private final ReadWriteLock lock = new ReentrantReadWriteLock();
    private Map<String, String> data = new HashMap<>();

    public String read(String key) {
        lock.readLock().lock();
        try {
            // Fixed: Actual read operation inside lock
            return data.get(key);
        } finally {
            lock.readLock().unlock();
        }
    }

    public void write(String key, String value) {
        lock.writeLock().lock();
        try {
            // Fixed: Actual write operation inside lock
            data.put(key, value);
        } finally {
            lock.writeLock().unlock();
        }
    }
}

// Fixed: Using AtomicInteger instead of synchronized blocks
import java.util.concurrent.atomic.AtomicInteger;

public class SecureAtomicCounter {
    private AtomicInteger counter = new AtomicInteger(0);

    public void increment() {
        // No synchronized block needed - atomic operation
        counter.incrementAndGet();
    }

    public int get() {
        return counter.get();
    }

    public int incrementAndGet() {
        return counter.incrementAndGet();
    }
}

// Pattern: Documenting synchronized blocks
public class DocumentedSync {
    private final Object dataLock = new Object();
    private String sensitiveData;
    private int accessCount;

    /**
     * Updates sensitive data atomically with access tracking.
     * Synchronization required to ensure atomicity of
     * data update and count increment.
     */
    public void updateData(String newData) {
        synchronized(dataLock) {
            // CRITICAL: Both operations must be atomic
            sensitiveData = newData;
            accessCount++;
            // END CRITICAL SECTION
        }
    }
}

---

CVE Examples

No specific CVEs are commonly attributed to this CWE, as it primarily affects code quality and reliability rather than direct security vulnerabilities.

---

References

1. MITRE Corporation. "CWE-585: Empty Synchronized Block." https://cwe.mitre.org/data/definitions/585.html
2. Java Concurrency in Practice by Brian Goetz.
3. FindBugs. "ESync: Empty synchronized block."