Back to all reviewers

Synchronization safety patterns

apache/kafka
Based on 6 comments
Java

Ensure proper synchronization mechanisms to prevent deadlocks and race conditions in concurrent code. When designing thread-safe components, carefully consider lock ordering, avoid nested synchronization blocks that could cause deadlocks, and make mutable shared state thread-safe.

Concurrency Java

Reviewer Prompt

Ensure proper synchronization mechanisms to prevent deadlocks and race conditions in concurrent code. When designing thread-safe components, carefully consider lock ordering, avoid nested synchronization blocks that could cause deadlocks, and make mutable shared state thread-safe.

Key practices:

  1. Avoid nested locks: Don’t call methods that acquire locks while holding another lock, as this can lead to deadlock scenarios
  2. Make shared mutable state thread-safe: Use proper synchronization for fields accessed by multiple threads
  3. Document concurrency assumptions: Clearly specify which methods are thread-safe and which require external synchronization
  4. Use appropriate synchronization primitives: Choose between synchronized blocks, ReentrantLock, or other mechanisms based on the specific use case

Example of problematic code:

// Dangerous: calling acquisitionLockTimeoutTask.run() within synchronized block
synchronized void completeStateTransition(boolean commit) {
    // ... other operations
    if (acquisitionLockTimeoutTask.hasExpired())
        acquisitionLockTimeoutTask.run(); // Could cause deadlock
}

// Dangerous: mutable public field accessed by multiple threads
public ClientInformation clientInformation; // Not thread-safe

Example of safer approach:

// Better: avoid nested locks, handle synchronization explicitly
synchronized void completeStateTransition(boolean commit) {
    // ... state transition logic
    // Schedule task execution outside of synchronized block
}

// Better: provide controlled access to mutable state
private volatile ClientInformation clientInformation;
public synchronized void updateClientInformation(ClientInformation info) {
    // Controlled update with proper synchronization
}

Always analyze the potential for deadlock when acquiring multiple locks and consider whether operations can be restructured to avoid nested synchronization.

6
Comments Analyzed
Java
Primary Language
Concurrency
Category

Source Discussions