Skip to content

Bug: ValidationTarget.cachedTargets is public static mutable HashMap with unsynchronized check-then-act #1621

Description

@jordanpadams

Summary

ValidationTarget.cachedTargets is declared public static HashMap<String, ValidationTarget>, making it fully accessible and mutable by any caller. The build() factory methods perform a containsKeyputget sequence without synchronization, creating a race window where two threads can both see containsKey return false, both call put, and the second overwrites the first.

The field's visibility is public, so callers outside LabelUtil have no enforced synchronization — yet LabelUtil.getIdentifiersCommon is synchronized specifically to avoid corrupting this map (per its Javadoc).

Affected code

gov/nasa/pds/tools/validate/ValidationTarget.java, line ~30:

public static HashMap<String, ValidationTarget> cachedTargets = new HashMap<>();

Recommended fix

Make cachedTargets private static and replace HashMap with ConcurrentHashMap. Use computeIfAbsent in build() to atomically eliminate the check-then-act race.

Related

Identified during immutability/synchronization audit triggered by PR #1607 review feedback.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    ToDo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions