From 70f8297cfacf8e0f4accb6f9a5341f66736dc822 Mon Sep 17 00:00:00 2001
From: Howard Gao <howard.gao@gmail.com>
Date: Tue, 25 Dec 2018 10:09:03 +0800
Subject: [PATCH] ARTEMIS-2210 PagingStore creation is not properly
 synchronized

In PagingManagerImpl#getPageStore() the operations on the map 'stores'
are not synchronzed and it's possible that more than one paging store is
created for one address.
---
 .../core/paging/impl/PagingManagerImpl.java   | 30 ++++++++++++-------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingManagerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingManagerImpl.java
index 357fda2ef6..92cbf031da 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingManagerImpl.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/impl/PagingManagerImpl.java
@@ -335,19 +335,30 @@ public final class PagingManagerImpl implements PagingManager {
    }
 
    /**
-    * stores is a ConcurrentHashMap, so we don't need to synchronize this method
+    * This method creates a new store if not exist.
     */
    @Override
    public PagingStore getPageStore(final SimpleString storeName) throws Exception {
       if (managementAddress != null && storeName.startsWith(managementAddress)) {
          return null;
       }
-      PagingStore store = stores.get(storeName);
 
+      PagingStore store = stores.get(storeName);
       if (store != null) {
          return store;
       }
-      return newStore(storeName);
+      //only if store is null we use computeIfAbsent
+      try {
+         return stores.computeIfAbsent(storeName, (s) -> {
+            try {
+               return newStore(s);
+            } catch (Exception e) {
+               throw new RuntimeException(e);
+            }
+         });
+      } catch (RuntimeException e) {
+         throw (Exception) e.getCause();
+      }
    }
 
    @Override
@@ -450,18 +461,15 @@ public final class PagingManagerImpl implements PagingManager {
       }
    }
 
+   //any caller that calls this method must guarantee the store doesn't exist.
    private PagingStore newStore(final SimpleString address) throws Exception {
       assert managementAddress == null || (managementAddress != null && !address.startsWith(managementAddress));
       syncLock.readLock().lock();
       try {
-         PagingStore store = stores.get(address);
-         if (store == null) {
-            store = pagingStoreFactory.newStore(address, addressSettingsRepository.getMatch(address.toString()));
-            store.start();
-            if (!cleanupEnabled) {
-               store.disableCleanup();
-            }
-            stores.put(address, store);
+         PagingStore store = pagingStoreFactory.newStore(address, addressSettingsRepository.getMatch(address.toString()));
+         store.start();
+         if (!cleanupEnabled) {
+            store.disableCleanup();
          }
          return store;
       } finally {