From 13a593848acd5cff387c6822f4efbe13a2800928 Mon Sep 17 00:00:00 2001
From: Justin Bertram <jbertram@apache.org>
Date: Wed, 4 Dec 2019 12:11:48 -0600
Subject: [PATCH] ARTEMIS-2566 race condition in concurrent dynamic queue
 lookup

---
 .../artemis/jndi/LazyCreateContext.java       | 23 ++++--
 .../activemq/artemis/jndi/JndiTest.java       | 78 +++++++++++++++++++
 2 files changed, 93 insertions(+), 8 deletions(-)
 create mode 100644 artemis-jms-client/src/test/java/org/apache/activemq/artemis/jndi/JndiTest.java

diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jndi/LazyCreateContext.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jndi/LazyCreateContext.java
index 52269d3dca..5b1a6262b9 100644
--- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jndi/LazyCreateContext.java
+++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jndi/LazyCreateContext.java
@@ -23,15 +23,22 @@ public abstract class LazyCreateContext extends ReadOnlyContext {
 
    @Override
    public Object lookup(String name) throws NamingException {
-      try {
-         return super.lookup(name);
-      } catch (NameNotFoundException e) {
-         Object answer = createEntry(name);
-         if (answer == null) {
-            throw e;
+      /*
+       * The lookup and the internalBind should be performed atomically to avoid
+       * race conditions between multiple threads where the lookup fails for both
+       * threads initially then the internalBind fails for the second thread.
+       */
+      synchronized (this) {
+         try {
+            return super.lookup(name);
+         } catch (NameNotFoundException e) {
+            Object answer = createEntry(name);
+            if (answer == null) {
+               throw e;
+            }
+            internalBind(name, answer);
+            return answer;
          }
-         internalBind(name, answer);
-         return answer;
       }
    }
 
diff --git a/artemis-jms-client/src/test/java/org/apache/activemq/artemis/jndi/JndiTest.java b/artemis-jms-client/src/test/java/org/apache/activemq/artemis/jndi/JndiTest.java
new file mode 100644
index 0000000000..ad20245607
--- /dev/null
+++ b/artemis-jms-client/src/test/java/org/apache/activemq/artemis/jndi/JndiTest.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.jndi;
+
+import javax.naming.Context;
+import javax.naming.InitialContext;
+import java.util.Hashtable;
+import java.util.UUID;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory.DYNAMIC_QUEUE_CONTEXT;
+
+public class JndiTest {
+
+   @Test
+   public void testMultiThreadedDynamicQueueLookup() throws Exception {
+      final int nThreads = 10;
+
+      LookerUpper[] lookerUppers = new LookerUpper[nThreads];
+      for (int j = 0; j < 100; j++) {
+         Hashtable<String, String> props = new Hashtable<>();
+         props.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
+         InitialContext context = new InitialContext(props);
+         String lookup = DYNAMIC_QUEUE_CONTEXT + "/" + UUID.randomUUID().toString();
+
+         for (int i = 0; i < nThreads; i++) {
+            lookerUppers[i] = new LookerUpper(context, lookup);
+         }
+
+         for (int i = 0; i < nThreads; i++) {
+            lookerUppers[i].start();
+         }
+
+         for (LookerUpper lookerUpper : lookerUppers) {
+            lookerUpper.join();
+            Assert.assertFalse(lookerUpper.failed);
+         }
+         context.close();
+      }
+   }
+
+   class LookerUpper extends Thread {
+      InitialContext context;
+      String lookup;
+      boolean failed = false;
+
+      LookerUpper(InitialContext context, String lookup) throws Exception {
+         this.context = context;
+         this.lookup = lookup;
+      }
+
+      @Override
+      public void run() {
+         try {
+            context.lookup(lookup);
+         } catch (Throwable e) {
+            failed = true;
+         }
+      }
+   }
+}