This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-configuration.git
commit 7ed54d7da52b762f0cb83cbfac820b9fd4ffcb42 Author: Gary Gregory <[email protected]> AuthorDate: Sat Aug 22 10:58:20 2026 -0400 Fix SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION in BasicConfigurationBuilder. --- src/changes/changes.xml | 1 + .../builder/BasicConfigurationBuilder.java | 61 ++++++++++++++-------- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 5211b4bbe..f415fa3e3 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -26,6 +26,7 @@ <release version="2.15.2" date="YYYY-MM-DD" description="This is a feature and maintenance release. Java 8 or later is required."> <!-- FIX --> <action type="fix" dev="ggregory" due-to="Gary Gregory">Add messages when throwing NullPointerException.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Fix SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION in BasicConfigurationBuilder.</action> <!-- ADD --> <!-- UPDATE --> <action type="update" dev="ggregory" due-to="Gary Gregory">Bump org.apache.commons:commons-parent from 100 to 103.</action> diff --git a/src/main/java/org/apache/commons/configuration2/builder/BasicConfigurationBuilder.java b/src/main/java/org/apache/commons/configuration2/builder/BasicConfigurationBuilder.java index 7c978f832..f3ca79c3a 100644 --- a/src/main/java/org/apache/commons/configuration2/builder/BasicConfigurationBuilder.java +++ b/src/main/java/org/apache/commons/configuration2/builder/BasicConfigurationBuilder.java @@ -130,6 +130,11 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme /** The result object of this builder. */ private volatile T result; + /** + * Private lock for synchronizing access. + */ + private final Object lock = new Object(); + /** * Creates a new instance of {@code BasicConfigurationBuilder} and initializes it with the given result class. No * initialization properties are set. @@ -192,13 +197,15 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * @param params The map with additional initialization parameters; may be <strong>null</strong>, then this call has no effect * @return A reference to this builder for method chaining */ - public synchronized BasicConfigurationBuilder<T> addParameters(final Map<String, Object> params) { - final Map<String, Object> newParams = new HashMap<>(getParameters()); - if (params != null) { - newParams.putAll(params); + public BasicConfigurationBuilder<T> addParameters(final Map<String, Object> params) { + synchronized (lock) { + final Map<String, Object> newParams = new HashMap<>(getParameters()); + if (params != null) { + newParams.putAll(params); + } + updateParameters(newParams); + return this; } - updateParameters(newParams); - return this; } /** @@ -260,7 +267,7 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * @param target The target configuration builder (must not be <strong>null</strong>) * @throws NullPointerException if the target builder is <strong>null</strong> */ - protected synchronized void copyEventListeners(final BasicConfigurationBuilder<?> target) { + protected void copyEventListeners(final BasicConfigurationBuilder<?> target) { copyEventListeners(target, eventListeners); } @@ -274,7 +281,9 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * @throws NullPointerException if the target builder is <strong>null</strong> */ protected void copyEventListeners(final BasicConfigurationBuilder<?> target, final EventListenerList listeners) { - target.eventListeners.addAll(listeners); + synchronized (lock) { + target.eventListeners.addAll(listeners); + } } /** @@ -443,8 +452,10 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * * @return A map with the current set of initialization parameters */ - protected final synchronized Map<String, Object> getParameters() { - return parameters; + protected final Map<String, Object> getParameters() { + synchronized (lock) { + return parameters; + } } /** @@ -463,11 +474,13 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * @return The {@code BeanDeclaration} for dynamically creating a result object * @throws ConfigurationException if an error occurs */ - protected final synchronized BeanDeclaration getResultDeclaration() throws ConfigurationException { - if (resultDeclaration == null) { - resultDeclaration = createResultDeclaration(getFilteredParameters()); + protected final BeanDeclaration getResultDeclaration() throws ConfigurationException { + synchronized (lock) { + if (resultDeclaration == null) { + resultDeclaration = createResultDeclaration(getFilteredParameters()); + } + return resultDeclaration; } - return resultDeclaration; } /** @@ -569,9 +582,11 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * Resets this builder. This is a convenience method which combines calls to {@link #resetResult()} and * {@link #resetParameters()}. */ - public synchronized void reset() { - resetParameters(); - resetResult(); + public void reset() { + synchronized (lock) { + resetParameters(); + resetResult(); + } } /** @@ -608,7 +623,7 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * parameters are removed * @return A reference to this builder for method chaining */ - public synchronized BasicConfigurationBuilder<T> setParameters(final Map<String, Object> params) { + public BasicConfigurationBuilder<T> setParameters(final Map<String, Object> params) { updateParameters(params); return this; } @@ -619,10 +634,12 @@ public class BasicConfigurationBuilder<T extends ImmutableConfiguration> impleme * @param newParams The map with new parameters (may be <strong>null</strong>) */ private void updateParameters(final Map<String, Object> newParams) { - final Map<String, Object> map = new HashMap<>(); - if (newParams != null) { - map.putAll(newParams); + synchronized (lock) { + final Map<String, Object> map = new HashMap<>(); + if (newParams != null) { + map.putAll(newParams); + } + parameters = Collections.unmodifiableMap(map); } - parameters = Collections.unmodifiableMap(map); } }
