Skip to content

Race condition on cacheModified flag in ToolchainDiscoverer from parallelStream() #173

Description

@elharo

Summary

ToolchainDiscoverer.getToolchainModel() is called from a parallelStream() in discoverToolchains(). The cacheModified volatile field is written concurrently by multiple threads without atomic coordination, which can cause cache updates to be lost.

Location

ToolchainDiscoverer.java:217-227

https://github.com/apache/maven-toolchains-plugin/blob/master/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java#L217-L227

Code

ToolchainModel getToolchainModel(Path jdk) {
    ToolchainModel model = cache.get(jdk);
    if (model == null) {
        model = doGetToolchainModel(jdk);
        if (model != null) {
            cache.put(jdk, model);
            cacheModified = true;
        }
    }
    return model;
}

Called from discoverToolchains() via parallel stream:

List<ToolchainModel> tcs = jdks.parallelStream()
        .map(s -> {
            ToolchainModel tc = getToolchainModel(s);
            // ...
        })
        .sorted(...)
        .collect(Collectors.toList());
writeCache();

Problem

  1. cacheModified = true is written from multiple threads in the parallel stream without synchronization
  2. After the parallel stream completes, writeCache() reads cacheModified, writes the cache file, and sets it back to false
  3. If a thread writes cacheModified = true after writeCache() has read it but before it sets it to false, the cache update is silently lost, and the newly discovered JDK won't be persisted
  4. The ConcurrentHashMap itself is fine (thread-safe), but the coordination between cacheModified and the cache file write is not atomic

Impact

Under concurrent load (which is the default for parallelStream()), some discovered JDK toolchains may not be persisted to the cache file. This means writeCache() may be called with cacheModified == false even though new entries were added during the stream.

Suggested Fix

Use an AtomicBoolean for cacheModified or re-check cache for new entries at write time:

private final AtomicBoolean cacheModified = new AtomicBoolean();

// In getToolchainModel:
cacheModified.set(true);

// In writeCache:
cacheModified.set(false);

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions