Repository: james-project Updated Branches: refs/heads/master 2db5e2fb0 -> 510b4e9b3
JAMES-2309 Long overflow in JMS delays. When using a Long.MAX as delay value the computeNextDeliveryTimestamp throws ArithmeticException caused by long overflow. Now we log the warning message and falling back to the maximum long value. Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/1b2539de Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/1b2539de Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/1b2539de Branch: refs/heads/master Commit: 1b2539de9700ed3fd03e5d4e28667ea4886a2ced Parents: 2db5e2f Author: Edgar Asatryan <nst...@gmail.com> Authored: Thu Jul 5 16:25:45 2018 +0400 Committer: benwa <btell...@linagora.com> Committed: Wed Jul 11 17:15:26 2018 +0700 ---------------------------------------------------------------------- .../activemq/ActiveMQMailQueueBlobTest.java | 16 +++++++------- .../queue/api/DelayedMailQueueContract.java | 2 +- .../apache/james/queue/jms/JMSMailQueue.java | 15 +++++++++---- .../queue/memory/MemoryMailQueueFactory.java | 22 ++++++++++++++++++-- 4 files changed, 41 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/1b2539de/server/queue/queue-activemq/src/test/java/org/apache/james/queue/activemq/ActiveMQMailQueueBlobTest.java ---------------------------------------------------------------------- diff --git a/server/queue/queue-activemq/src/test/java/org/apache/james/queue/activemq/ActiveMQMailQueueBlobTest.java b/server/queue/queue-activemq/src/test/java/org/apache/james/queue/activemq/ActiveMQMailQueueBlobTest.java index 36dfa8b..3cb606e 100644 --- a/server/queue/queue-activemq/src/test/java/org/apache/james/queue/activemq/ActiveMQMailQueueBlobTest.java +++ b/server/queue/queue-activemq/src/test/java/org/apache/james/queue/activemq/ActiveMQMailQueueBlobTest.java @@ -18,11 +18,13 @@ ****************************************************************/ package org.apache.james.queue.activemq; +import static org.assertj.core.api.Assertions.assertThat; + import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; import org.apache.activemq.ActiveMQConnectionFactory; import org.apache.activemq.broker.BrokerService; @@ -110,17 +112,17 @@ public class ActiveMQMailQueueBlobTest implements DelayedManageableMailQueueCont @Test @Override - @Disabled("JAMES-2309 Long overflow in JMS delays") - public void enqueueWithVeryLongDelayShouldDelayMail(ExecutorService executorService) { + @Disabled("JAMES-2312 JMS clear mailqueue can ommit some messages" + + "Random test failing around 1% of the time") + public void clearShouldRemoveAllElements() { } @Test - @Override - @Disabled("JAMES-2312 JMS clear mailqueue can ommit some messages" + - "Random test failing around 1% of the time") - public void clearShouldRemoveAllElements() { + void computeNextDeliveryTimestampShouldReturnLongMaxWhenOverflow() { + long deliveryTimestamp = mailQueue.computeNextDeliveryTimestamp(Long.MAX_VALUE, TimeUnit.DAYS); + assertThat(deliveryTimestamp).isEqualTo(Long.MAX_VALUE); } protected ActiveMQConnectionFactory createConnectionFactory() { http://git-wip-us.apache.org/repos/asf/james-project/blob/1b2539de/server/queue/queue-api/src/test/java/org/apache/james/queue/api/DelayedMailQueueContract.java ---------------------------------------------------------------------- diff --git a/server/queue/queue-api/src/test/java/org/apache/james/queue/api/DelayedMailQueueContract.java b/server/queue/queue-api/src/test/java/org/apache/james/queue/api/DelayedMailQueueContract.java index 2744d77..6e48232 100644 --- a/server/queue/queue-api/src/test/java/org/apache/james/queue/api/DelayedMailQueueContract.java +++ b/server/queue/queue-api/src/test/java/org/apache/james/queue/api/DelayedMailQueueContract.java @@ -79,7 +79,7 @@ public interface DelayedMailQueueContract { default void enqueueWithVeryLongDelayShouldDelayMail(ExecutorService executorService) throws Exception { getMailQueue().enQueue(defaultMail() .build(), - 100 * 365, + Long.MAX_VALUE, TimeUnit.DAYS); Future<?> future = executorService.submit(Throwing.runnable(() -> getMailQueue().deQueue())); http://git-wip-us.apache.org/repos/asf/james-project/blob/1b2539de/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java ---------------------------------------------------------------------- diff --git a/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java b/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java index e6a6958..08e9e11 100644 --- a/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java +++ b/server/queue/queue-jms/src/main/java/org/apache/james/queue/jms/JMSMailQueue.java @@ -255,10 +255,17 @@ public class JMSMailQueue implements ManageableMailQueue, JMSSupport, MailPriori public long computeNextDeliveryTimestamp(long delay, TimeUnit unit) { if (delay > 0) { - return ZonedDateTime.now() - .plus(delay, Temporals.chronoUnit(unit)) - .toInstant() - .toEpochMilli(); + try { + return ZonedDateTime.now() + .plus(delay, Temporals.chronoUnit(unit)) + .toInstant() + .toEpochMilli(); + } catch (ArithmeticException e) { + LOGGER.warn("The {} was caused by conversation {}({}) followed by addition to current timestamp. Falling back to Long.MAX_VALUE.", + e.getMessage(), delay, unit.name()); + + return Long.MAX_VALUE; + } } return NO_DELAY; } http://git-wip-us.apache.org/repos/asf/james-project/blob/1b2539de/server/queue/queue-memory/src/main/java/org/apache/james/queue/memory/MemoryMailQueueFactory.java ---------------------------------------------------------------------- diff --git a/server/queue/queue-memory/src/main/java/org/apache/james/queue/memory/MemoryMailQueueFactory.java b/server/queue/queue-memory/src/main/java/org/apache/james/queue/memory/MemoryMailQueueFactory.java index a5b4233..6120272 100644 --- a/server/queue/queue-memory/src/main/java/org/apache/james/queue/memory/MemoryMailQueueFactory.java +++ b/server/queue/queue-memory/src/main/java/org/apache/james/queue/memory/MemoryMailQueueFactory.java @@ -19,6 +19,8 @@ package org.apache.james.queue.memory; +import java.time.Instant; +import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.Iterator; import java.util.Optional; @@ -98,7 +100,7 @@ public class MemoryMailQueueFactory implements MailQueueFactory<ManageableMailQu @Override public void enQueue(Mail mail, long delay, TimeUnit unit) throws MailQueueException { - ZonedDateTime nextDelivery = ZonedDateTime.now().plus(delay, Temporals.chronoUnit(unit)); + ZonedDateTime nextDelivery = calculateNextDelivery(delay, unit); try { mailItems.put(new MemoryMailQueueItem(cloneMail(mail), this, nextDelivery)); } catch (MessagingException e) { @@ -106,6 +108,18 @@ public class MemoryMailQueueFactory implements MailQueueFactory<ManageableMailQu } } + private ZonedDateTime calculateNextDelivery(long delay, TimeUnit unit) { + if (delay > 0) { + try { + return ZonedDateTime.now().plus(delay, Temporals.chronoUnit(unit)); + } catch (ArithmeticException e) { + return Instant.ofEpochMilli(Long.MAX_VALUE).atZone(ZoneId.of("UTC")); + } + } + + return ZonedDateTime.now(); + } + @Override public void enQueue(Mail mail) throws MailQueueException { enQueue(mail, 0, TimeUnit.SECONDS); @@ -256,7 +270,11 @@ public class MemoryMailQueueFactory implements MailQueueFactory<ManageableMailQu @Override public long getDelay(TimeUnit unit) { - return ZonedDateTime.now().until(delivery, Temporals.chronoUnit(unit)); + try { + return ZonedDateTime.now().until(delivery, Temporals.chronoUnit(unit)); + } catch (ArithmeticException e) { + return Long.MAX_VALUE; + } } @Override --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
