From 1a077348e2d06a541f2ef6cc94342a299220ff72 Mon Sep 17 00:00:00 2001 From: Robbie Gemmell Date: Fri, 22 Sep 2017 16:33:21 +0100 Subject: [PATCH] ARTEMIS-1434: update handling of tick deadline values - account for potential to be negative due to using nanoTime derived values - add some other edge case protections to avoid task ceasing in error --- .../amqp/proton/AMQPConnectionContext.java | 8 ++++---- .../transport/amqp/client/AmqpConnection.java | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java index 47cb11dca1..680111a477 100644 --- a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java +++ b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java @@ -387,13 +387,13 @@ public class AMQPConnectionContext extends ProtonInitializable implements EventH * */ if (connection.getRemoteProperties() == null || !connection.getRemoteProperties().containsKey(CONNECTION_OPEN_FAILED)) { long nextKeepAliveTime = handler.tick(true); - if (nextKeepAliveTime > 0 && scheduledPool != null) { + if (nextKeepAliveTime != 0 && scheduledPool != null) { scheduledPool.schedule(new Runnable() { @Override public void run() { - long rescheduleAt = (handler.tick(false) - TimeUnit.NANOSECONDS.toMillis(System.nanoTime())); - if (rescheduleAt > 0) { - scheduledPool.schedule(this, rescheduleAt, TimeUnit.MILLISECONDS); + long rescheduleAt = handler.tick(false); + if (rescheduleAt != 0) { + scheduledPool.schedule(this, rescheduleAt - TimeUnit.NANOSECONDS.toMillis(System.nanoTime()), TimeUnit.MILLISECONDS); } } }, (nextKeepAliveTime - TimeUnit.NANOSECONDS.toMillis(System.nanoTime())), TimeUnit.MILLISECONDS); diff --git a/tests/artemis-test-support/src/main/java/org/apache/activemq/transport/amqp/client/AmqpConnection.java b/tests/artemis-test-support/src/main/java/org/apache/activemq/transport/amqp/client/AmqpConnection.java index 84e7ba14af..2fc720abbf 100644 --- a/tests/artemis-test-support/src/main/java/org/apache/activemq/transport/amqp/client/AmqpConnection.java +++ b/tests/artemis-test-support/src/main/java/org/apache/activemq/transport/amqp/client/AmqpConnection.java @@ -606,7 +606,7 @@ public class AmqpConnection extends AmqpAbstractResource implements // Using nano time since it is not related to the wall clock, which may change long initialNow = TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); long initialKeepAliveDeadline = protonTransport.tick(initialNow); - if (initialKeepAliveDeadline > 0) { + if (initialKeepAliveDeadline != 0) { getScheduler().schedule(new Runnable() { @@ -617,15 +617,15 @@ public class AmqpConnection extends AmqpAbstractResource implements LOG.debug("Client performing next idle check"); // Using nano time since it is not related to the wall clock, which may change long now = TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); - long rescheduleAt = protonTransport.tick(now) - now; + long deadline = protonTransport.tick(now); pumpToProtonTransport(); if (protonTransport.isClosed()) { LOG.debug("Transport closed after inactivity check."); - throw new InactivityIOException("Channel was inactive for to long"); - } - - if (rescheduleAt > 0) { - getScheduler().schedule(this, rescheduleAt, TimeUnit.MILLISECONDS); + throw new InactivityIOException("Channel was inactive for too long"); + } else { + if (deadline != 0) { + getScheduler().schedule(this, deadline - now, TimeUnit.MILLISECONDS); + } } } } catch (Exception e) {