From 40a6bab8989869670797bffdadf53b731be31e54 Mon Sep 17 00:00:00 2001
From: Justin Bertram <jbertram@apache.org>
Date: Mon, 13 Apr 2020 11:10:39 -0500
Subject: [PATCH] ARTEMIS-2708 JDK bug causes missed props reload

---
 .../apache/activemq/cli/test/ArtemisTest.java | 39 +++++++++++++++++++
 .../security/jaas/ReloadableProperties.java   |  9 ++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java
index 97d910a77b..92f7c3d0ae 100644
--- a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java
+++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java
@@ -593,6 +593,45 @@ public class ArtemisTest extends CliTestBase {
       stopServer();
    }
 
+   @Test
+   public void testProperReloadWhenAddingUserViaManagement() throws Exception {
+      Run.setEmbedded(true);
+      File instance1 = new File(temporaryFolder.getRoot(), "instance_user");
+      System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config");
+      Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--no-amqp-acceptor", "--no-mqtt-acceptor", "--no-stomp-acceptor", "--no-hornetq-acceptor", "--require-login");
+      System.setProperty("artemis.instance", instance1.getAbsolutePath());
+      Object result = Artemis.internalExecute("run");
+      ActiveMQServer activeMQServer = ((Pair<ManagementContext, ActiveMQServer>)result).getB();
+      ActiveMQServerControl activeMQServerControl = activeMQServer.getActiveMQServerControl();
+
+      ServerLocator serverLocator = ActiveMQClient.createServerLocator("tcp://localhost:61616");
+      ClientSessionFactory sessionFactory = serverLocator.createSessionFactory();
+
+      try {
+         // this will force a properties "reload" event (i.e. initial loading)
+         sessionFactory.createSession("foo", "bar", false, false, false, false, 0);
+         fail("Should have failed to create session here due to security");
+      } catch (Exception e) {
+         // ignore
+      }
+
+      try {
+         activeMQServerControl.createAddress("myAddress", RoutingType.ANYCAST.toString());
+         activeMQServerControl.addSecuritySettings("myAddress", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole", "myRole");
+         // change properties files which should cause another "reload" event
+         activeMQServerControl.addUser("foo", "bar", "myRole", true);
+         ClientSession session = sessionFactory.createSession("foo", "bar", false, false, false, false, 0);
+         session.createQueue("myAddress", RoutingType.ANYCAST, "myQueue", true);
+         ClientProducer producer = session.createProducer("myAddress");
+         producer.send(session.createMessage(true));
+         session.close();
+      } finally {
+         sessionFactory.close();
+         serverLocator.close();
+         stopServer();
+      }
+   }
+
    @Test
    public void testMissingUserFileViaManagement() throws Exception {
       Run.setEmbedded(true);
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java
index 947c6d164b..35c747df7e 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java
@@ -137,7 +137,14 @@ public class ReloadableProperties {
    }
 
    private boolean hasModificationAfter(long reloadTime) {
-      return key.file.lastModified() > reloadTime;
+      /**
+       * A bug in JDK 8/9 (i.e. https://bugs.openjdk.java.net/browse/JDK-8177809) causes java.io.File.lastModified() to
+       * lose resolution past 1 second. Because of this, the value returned by java.io.File.lastModified() can appear to
+       * be smaller than it actually is which can cause the broker to miss reloading the properties if the modification
+       * happens close to another "reload" event (e.g. initial loading). In order to *not* miss file modifications that
+       * need to be reloaded we artificially inflate the value returned by java.io.File.lastModified() by 1 second.
+       */
+      return key.file.lastModified() + 1000 > reloadTime;
    }
 
    private boolean looksLikeRegexp(String str) {