Check Concurrency Skill

Check Concurrency Skill

Identify concurrency bugs, race conditions, deadlocks, and thread safety issues in the codebase.

Instructions

1. Identify Shared Mutable State

Search for mutable state that may be accessed by multiple threads:

Instance/​Static Fields:

// Dangerous - mutable shared state
private Map<K, V> cache = new HashMap<>();
private List<T> items = new ArrayList<>();
private int counter = 0;
private boolean flag = false;

// Safe alternatives
private final ConcurrentHashMap<K, V> cache = new ConcurrentHashMap<>();
private final CopyOnWriteArrayList<T> items = new CopyOnWriteArrayList<>();
private final AtomicInteger counter = new AtomicInteger();
private final AtomicBoolean flag = new AtomicBoolean();

Use Grep to find:

private.*Map<
private.*List<
private.*Set<
private static(?!.*final)
private.*= 0;
private.*= false;
private.*= true;
private.*= null;

2. Check for Race Conditions

Check-Then-Act Anti-patterns:

// RACE CONDITION: check and act not atomic
if (!map.containsKey(key)) {
    map.put(key, value);  // Another thread may have put between check and act
}

// SAFE: Use atomic operations
map.putIfAbsent(key, value);
map.computeIfAbsent(key, k -> createValue());

Read-Modify-Write Anti-patterns:

// RACE CONDITION: increment not atomic
counter++;
counter = counter + 1;

// SAFE: Use atomic types
atomicCounter.incrementAndGet();

Lazy Initialization:

// RACE CONDITION: double-checked locking without volatile
if (instance == null) {
    synchronized(lock) {
        if (instance == null) {
            instance = new Instance();  // May return partially constructed
        }
    }
}

// SAFE: Use volatile or holder pattern
private static volatile Instance instance;
// or
private static class Holder {
    static final Instance INSTANCE = new Instance();
}

3. Detect Potential Deadlocks

Lock Ordering Issues:

// DEADLOCK RISK: Different lock ordering
// Thread 1: synchronized(lockA) { synchronized(lockB) { ... } }
// Thread 2: synchronized(lockB) { synchronized(lockA) { ... } }

Search for nested synchronization:

synchronized.*\{[\s\S]*synchronized

Resource Acquisition:

// DEADLOCK RISK: Acquiring multiple semaphores
semaphoreA.acquire();
semaphoreB.acquire();  // What if another thread holds B, wants A?

4. Analyze Synchronization Usage

Over-synchronization (performance issue):

// BAD: Synchronized on entire method when only part needs it
public synchronized void process(Data data) {
    // ... lots of non-shared work ...
    cache.put(key, value);  // Only this needs sync
}

// BETTER: Minimize synchronized scope
public void process(Data data) {
    // ... non-shared work ...
    synchronized(lock) {
        cache.put(key, value);
    }
}

Under-synchronization (correctness issue):

// BAD: Some methods synchronized, some not
public synchronized void add(T item) { list.add(item); }
public T get(int i) { return list.get(i); }  // NOT synchronized!

5. Check Thread-Safe Collections Usage

Compound Operations on Concurrent Collections:

ConcurrentHashMap<K, V> map = ...;

// RACE CONDITION: Compound operation not atomic
V value = map.get(key);
if (value == null) {
    value = computeValue();
    map.put(key, value);  // Another thread may have computed too
}

// SAFE: Use atomic compute methods
V value = map.computeIfAbsent(key, k -> computeValue());

Iteration Safety:

// UNSAFE: ConcurrentModificationException or stale data
for (K key : map.keySet()) {
    map.remove(key);  // Modifying during iteration
}

// SAFE: Use iterator.remove() or collect keys first

6. Virtual Threads & StructuredTaskScope (Java 21+)

This project uses Java 24 virtual threads. Check for:

Pinning Issues:

// BAD: synchronized pins virtual thread to carrier
synchronized(lock) {
    blockingCall();  // Virtual thread pinned!
}

// BETTER: Use ReentrantLock
lock.lock();
try {
    blockingCall();  // Virtual thread can unmount
} finally {
    lock.unlock();
}

StructuredTaskScope Usage:

// Check for proper scope management
try (var scope = new StructuredTaskScope.ShutdownOnFailure()) {
    var future1 = scope.fork(() -> task1());
    var future2 = scope.fork(() -> task2());
    scope.join();           // Must join before getting results
    scope.throwIfFailed();  // Must check for failures
    return combine(future1.get(), future2.get());
}

// ISSUES TO CHECK:
// - Missing join() before accessing results
// - Missing throwIfFailed() for ShutdownOnFailure
// - Scope not closed (try-with-resources required)
// - Sharing scope between threads

7. Reactive/​WebFlux Concurrency

Publisher Sharing:

// UNSAFE: Sharing mutable state in reactive chain
List<String> results = new ArrayList<>();  // Shared mutable!
flux.doOnNext(item -> results.add(item))   // Race condition!
    .subscribe();

// SAFE: Use collectList() or reduce()
flux.collectList().subscribe(results -> ...);

Scheduler Awareness:

// Check that shared state access considers scheduler
mono.publishOn(Schedulers.parallel())  // Multiple threads!
    .map(data -> {
        sharedState.update(data);  // RACE CONDITION
        return data;
    });

8. This Project's Specific Patterns

Files to examine:

• AggregatorService.java - Uses StructuredTaskScope, Semaphores, ConcurrentHashMap
• CurrentConditionsService.java - Caching with concurrent access
• ForecastService.java - Data parsing with potential shared state
• *Strategy.java - Strategy implementations called concurrently

Known concurrent structures in this project:

// Check these are used correctly:
ConcurrentHashMap<Integer, ForecastData> forecastCache
ConcurrentHashMap<Integer, CurrentConditions> currentConditions
AtomicReference<List<Spot>> spots
Semaphore (for rate limiting)

9. Common Concurrency Anti-patterns

Output Format

## Concurrency Analysis Report

### Summary
| Category | Issues Found | Severity |
|----------|--------------|----------|
| Race Conditions | X | Critical |
| Potential Deadlocks | X | Critical |
| Unsafe Shared State | X | High |
| Synchronization Issues | X | Medium |
| Virtual Thread Issues | X | Medium |

### Critical Issues

#### Race Condition: [Description]
**File**: `path/​to/​file.java:line`
**Pattern**: Check-then-act on HashMap
**Threads Involved**: Scheduler thread, Request threads
```java
// Current code
if (!cache.containsKey(key)) {
    cache.put(key, expensiveCompute());
}

Risk: Duplicate computation, inconsistent state Fix:

cache.computeIfAbsent(key, k -> expensiveCompute());

Potential Deadlock: [Description]

File: path/​to/​file.java:line Pattern: Nested locks with inconsistent ordering Fix: Establish global lock ordering or use tryLock with timeout

High Priority Issues

Unsafe Shared State

Medium Priority Issues

Synchronization Improvements

• File.java:42 - Consider narrowing synchronized scope
• File.java:78 - Missing volatile on field read by multiple threads

Verified Thread-Safe Patterns

StructuredTaskScope Analysis

Recommendations

1. Immediate: Fix race condition in Cache.java:42
2. Review: Audit all HashMap usages for thread safety
3. Consider: Add @ThreadSafe/​@NotThreadSafe annotations for documentation
4. Testing: Add concurrent stress tests for critical sections

## Execution Steps

1. Use `Grep` to find mutable field declarations
2. Use `Grep` to find `synchronized`, `Lock`, `Semaphore` usage
3. Use `Grep` to find concurrent collection usage
4. Read files to analyze compound operations
5. Check StructuredTaskScope for proper join/​close
6. Look for check-then-act and read-modify-write patterns
7. Analyze lock ordering for deadlock potential
8. Generate categorized report

## Notes

- Virtual threads change some concurrency patterns (synchronized pins carrier thread)
- ConcurrentHashMap is safe for individual operations, not compound ones
- AtomicReference doesn't make the referenced object thread-safe
- Reactive chains may execute on different threads at different stages
- `@Scheduled` methods may run concurrently if previous execution is slow
- Focus on request-handling code paths over initialization code