[core] Fix lazy init for allSuppressors in DefaultRuleViolationFactory

Fixes #4035
This commit is contained in:
Andreas Dangel
2022-07-22 12:23:13 +02:00
parent 8bf54b8a68
commit 2f1462b621
2 changed files with 17 additions and 7 deletions

View File

@ -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

View File

@ -26,7 +26,9 @@ public class DefaultRuleViolationFactory implements RuleViolationFactory {
// todo move to package reporting
private static final DefaultRuleViolationFactory DEFAULT = new DefaultRuleViolationFactory();
private Set<ViolationSuppressor> allSuppressors;
// volatile for lazy init - see #getAllSuppressors
private volatile Set<ViolationSuppressor> 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<ViolationSuppressor> getAllSuppressors() {
if (allSuppressors == null) {
Set<ViolationSuppressor> 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). */