diff --git a/docs/pages/7_0_0_release_notes.md b/docs/pages/7_0_0_release_notes.md index be94436737..de1b50f21d 100644 --- a/docs/pages/7_0_0_release_notes.md +++ b/docs/pages/7_0_0_release_notes.md @@ -184,6 +184,8 @@ The following previously deprecated rules have been finally removed: * miscellaneous * [#896](https://github.com/pmd/pmd/issues/896): \[all] Use slf4j * [#1451](https://github.com/pmd/pmd/issues/1451): \[core] RulesetFactoryCompatibility stores the whole ruleset file in memory as a string +* core + * [#4035](https://github.com/pmd/pmd/issues/4035): \[core] ConcurrentModificationException in DefaultRuleViolationFactory * cli * [#3828](https://github.com/pmd/pmd/issues/3828): \[core] Progress reporting * apex-design diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java index e44088831b..be1560ed2d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java @@ -26,7 +26,9 @@ public class DefaultRuleViolationFactory implements RuleViolationFactory { // todo move to package reporting private static final DefaultRuleViolationFactory DEFAULT = new DefaultRuleViolationFactory(); - private Set allSuppressors; + + // volatile for lazy init - see #getAllSuppressors + private volatile Set allSuppressors; // NOPMD volatile needed for lazy init @Override public SuppressedViolation suppressOrNull(Node location, RuleViolation violation) { @@ -48,16 +50,22 @@ public class DefaultRuleViolationFactory implements RuleViolationFactory { } private Set getAllSuppressors() { - if (allSuppressors == null) { + Set result = allSuppressors; + if (result == null) { // lazy loaded because calling getSuppressors in constructor // is not safe wrt initialization of static constants // (order dependent otherwise) - this.allSuppressors = new LinkedHashSet<>(getSuppressors()); - allSuppressors.add(ViolationSuppressor.NOPMD_COMMENT_SUPPRESSOR); - allSuppressors.add(ViolationSuppressor.REGEX_SUPPRESSOR); - allSuppressors.add(ViolationSuppressor.XPATH_SUPPRESSOR); + result = new LinkedHashSet<>(getSuppressors()); + result.add(ViolationSuppressor.NOPMD_COMMENT_SUPPRESSOR); + result.add(ViolationSuppressor.REGEX_SUPPRESSOR); + result.add(ViolationSuppressor.XPATH_SUPPRESSOR); + + // note 1: allSuppressors must be volatile to avoid other threads seeing the HashSet under construction + // note 2: multiple threads might create their own HashSets and the last HashSet is stored and overwrites + // previously created HashSets. This is ok, because this method is supposed to be idempotent. + allSuppressors = result; } - return allSuppressors; + return result; } /** Returns the default instance (no additional suppressors, creates a ParametricRuleViolation). */