This is an automated email from the ASF dual-hosted git repository.

jbonofre pushed a commit to branch activemq-6.2.x
in repository https://gitbox.apache.org/repos/asf/activemq.git


The following commit(s) were added to refs/heads/activemq-6.2.x by this push:
     new df8dbdf2ee AMQ9844 Refactor ReloadableProperties and 
PropertiesLoginModuleRaceConditionTest to improve error handling and 
concurrency management (#1636)
df8dbdf2ee is described below

commit df8dbdf2ee2615a5359fd58ccab16b8d10a409a4
Author: Jean-Louis Monteiro <[email protected]>
AuthorDate: Sun Feb 1 16:49:02 2026 +0100

    AMQ9844 Refactor ReloadableProperties and 
PropertiesLoginModuleRaceConditionTest to improve error handling and 
concurrency management (#1636)
---
 .../apache/activemq/jaas/ReloadableProperties.java | 23 ++++++----
 .../PropertiesLoginModuleRaceConditionTest.java    | 49 +++++++++++-----------
 2 files changed, 39 insertions(+), 33 deletions(-)

diff --git 
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/ReloadableProperties.java
 
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/ReloadableProperties.java
index 9950bdf0af..51cc8ea4d3 100644
--- 
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/ReloadableProperties.java
+++ 
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/ReloadableProperties.java
@@ -49,19 +49,25 @@ public class ReloadableProperties {
 
     public synchronized ReloadableProperties obtained() {
         if (reloadTime < 0 || (key.isReload() && 
hasModificationAfter(reloadTime))) {
-            props = new Properties();
+            // Load into a local variable first to preserve old data if reload 
fails
+            // If we assigned 'props = new Properties()' first and load() 
throws IOException,
+            // we'd expose empty Properties. By loading into newProps first, 
we preserve the
+            // old data if the reload fails.
+            final Properties newProps = new Properties();
             try {
-                load(key.file(), props);
+                load(key.file(), newProps);
+                // Only assign to the instance field after successful load
+                props = newProps;
                 invertedProps = null;
                 invertedValueProps = null;
                 regexpProps = null;
                 if (key.isDebug()) {
-                    LOG.debug("Load of: " + key);
+                    LOG.debug("Load of: {}", key);
                 }
             } catch (IOException e) {
-                LOG.error("Failed to load: " + key + ", reason:" + 
e.getLocalizedMessage());
+                LOG.error("Failed to load: {}, reason:{}", key, 
e.getLocalizedMessage());
                 if (key.isDebug()) {
-                    LOG.debug("Load of: " + key + ", failure exception" + e);
+                    LOG.debug("Load of: {}, failure exception{}", key, e);
                 }
             }
             reloadTime = System.currentTimeMillis();
@@ -119,14 +125,15 @@ public class ReloadableProperties {
     }
 
     private void load(final File source, Properties props) throws IOException {
-        FileInputStream in = new FileInputStream(source);
+        final FileInputStream in = new FileInputStream(source);
         try {
             props.load(in);
             if (key.isDecrypt()) {
                 try {
-                    EncryptionSupport.decrypt(this.props, key.getAlgorithm());
+                    // Decrypt the parameter props, not this.props (which may 
be the old instance)
+                    EncryptionSupport.decrypt(props, key.getAlgorithm());
                 } catch (NoClassDefFoundError e) {
-                    // this Happens whe jasypt is not on the classpath..
+                    // this Happens when jasypt is not on the classpath..
                     key.setDecrypt(false);
                     LOG.info("jasypt is not on the classpath: password 
decryption disabled.");
                 }
diff --git 
a/activemq-jaas/src/test/java/org/apache/activemq/jaas/PropertiesLoginModuleRaceConditionTest.java
 
b/activemq-jaas/src/test/java/org/apache/activemq/jaas/PropertiesLoginModuleRaceConditionTest.java
index b401bd82bc..f8383b1e3a 100644
--- 
a/activemq-jaas/src/test/java/org/apache/activemq/jaas/PropertiesLoginModuleRaceConditionTest.java
+++ 
b/activemq-jaas/src/test/java/org/apache/activemq/jaas/PropertiesLoginModuleRaceConditionTest.java
@@ -49,6 +49,7 @@ public class PropertiesLoginModuleRaceConditionTest {
     private static final String USERS_FILE = "users.properties";
     private static final String USERNAME = "first";
     private static final String PASSWORD = "secret";
+    private static final int TOTAL_LOGIN_ATTEMPTS = 25000;
 
     @Rule
     public final ErrorCollector e = new ErrorCollector();
@@ -112,7 +113,8 @@ public class PropertiesLoginModuleRaceConditionTest {
         options.put("org.apache.activemq.jaas.properties.group", GROUPS_FILE);
         options.put("baseDir", temp.getRoot().getAbsolutePath());
 
-        errors = new ArrayBlockingQueue<Exception>(processorCount());
+        // Large enough to hold all potential errors from concurrent attempts
+        errors = new ArrayBlockingQueue<Exception>(TOTAL_LOGIN_ATTEMPTS);
         pool = Executors.newFixedThreadPool(processorCount());
         callback = new JassCredentialCallbackHandler(USERNAME, PASSWORD);
     }
@@ -128,24 +130,27 @@ public class PropertiesLoginModuleRaceConditionTest {
     public void raceConditionInUsersAndGroupsLoading() throws 
InterruptedException, FileNotFoundException, IOException {
 
         // Brute force approach to increase the likelihood of the race 
condition occurring
-        for (int i = 0; i < 25000; i++) {
-            final CountDownLatch start = new CountDownLatch(1);
-            final CountDownLatch finished = new 
CountDownLatch(processorCount());
-            prepareLoginThreads(start, finished);
-
-            // Releases every login thread simultaneously to increase our 
chances of
-            // encountering the race condition
-            start.countDown();
-
-            finished.await();
-            if (isRaceConditionDetected()) {
-                e.addError(new AssertionError("At least one race condition in 
PropertiesLoginModule "
-                        + "has been encountered. Please examine the "
-                        + "following stack traces for more details:"));
-                for (Exception exception : errors) {
-                    e.addError(exception);
-                }
-                return;
+        // Submit many concurrent login attempts and let the thread pool 
handle concurrency
+        final CountDownLatch start = new CountDownLatch(1);
+        final CountDownLatch finished = new 
CountDownLatch(TOTAL_LOGIN_ATTEMPTS);
+
+        // Submit all login tasks to the pool
+        for (int i = 0; i < TOTAL_LOGIN_ATTEMPTS; i++) {
+            pool.submit(new LoginTester(start, finished, errors, options, 
callback));
+        }
+
+        // Release all threads simultaneously to maximize concurrent load
+        start.countDown();
+
+        // Wait for all attempts to complete
+        finished.await();
+
+        if (isRaceConditionDetected()) {
+            e.addError(new AssertionError("At least one race condition in 
PropertiesLoginModule "
+                    + "has been encountered. Please examine the "
+                    + "following stack traces for more details:"));
+            for (Exception exception : errors) {
+                e.addError(exception);
             }
         }
     }
@@ -154,12 +159,6 @@ public class PropertiesLoginModuleRaceConditionTest {
         return errors.size() > 0;
     }
 
-    private void prepareLoginThreads(final CountDownLatch start, final 
CountDownLatch finished) {
-        for (int processor = 1; processor <= processorCount() * 2; 
processor++) {
-            pool.submit(new LoginTester(start, finished, errors, options, 
callback));
-        }
-    }
-
     private int processorCount() {
         return Runtime.getRuntime().availableProcessors();
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact


Reply via email to