diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java index 2088673d2c..a8448bb0a2 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/critical/CriticalMeasure.java @@ -17,7 +17,6 @@ package org.apache.activemq.artemis.utils.critical; -import java.util.concurrent.atomic.AtomicLongFieldUpdater; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import org.jboss.logging.Logger; @@ -29,9 +28,6 @@ public class CriticalMeasure { // this is used on enterCritical, if the logger is in trace mode private volatile Exception traceEnter; - //uses updaters to avoid creates many AtomicLong instances - static final AtomicLongFieldUpdater TIME_ENTER_UPDATER = AtomicLongFieldUpdater.newUpdater(CriticalMeasure.class, "timeEnter"); - static final AtomicReferenceFieldUpdater CURRENT_THREAD_UDPATER = AtomicReferenceFieldUpdater.newUpdater(CriticalMeasure.class, Thread.class, "currentThread"); // While resetting the leaveMethod, I want to make sure no enter call would reset the value. @@ -39,7 +35,7 @@ public class CriticalMeasure { private static final Thread GHOST_THREAD = new Thread(); private volatile Thread currentThread; - private volatile long timeEnter; + protected volatile long timeEnter; private final int id; private final CriticalComponent component; @@ -47,7 +43,7 @@ public class CriticalMeasure { public CriticalMeasure(CriticalComponent component, int id) { this.id = id; this.component = component; - TIME_ENTER_UPDATER.set(this, 0); + this.timeEnter = 0; } public void enterCritical() { @@ -55,8 +51,7 @@ public class CriticalMeasure { // a sampling of a single thread at a time will be sufficient for the analyser, // typically what causes one thread to stall will repeat on another if (CURRENT_THREAD_UDPATER.compareAndSet(this, null, Thread.currentThread())) { - //prefer lazySet in order to avoid heavy-weight full barriers on x86 - TIME_ENTER_UPDATER.lazySet(this, System.nanoTime()); + timeEnter = System.nanoTime(); if (logger.isTraceEnabled()) { traceEnter = new Exception("entered"); @@ -83,7 +78,7 @@ public class CriticalMeasure { } traceEnter = null; } - TIME_ENTER_UPDATER.set(this, 0); + this.timeEnter = 0; // I am pretty sure this is single threaded by now.. I don't need compareAndSet here CURRENT_THREAD_UDPATER.set(this, null); @@ -99,7 +94,7 @@ public class CriticalMeasure { } public boolean checkExpiration(long timeout, boolean reset) { - final long timeEnter = TIME_ENTER_UPDATER.get(this); + final long timeEnter = this.timeEnter; if (timeEnter != 0L) { long time = System.nanoTime(); boolean expired = time - timeEnter > timeout; @@ -113,13 +108,13 @@ public class CriticalMeasure { logger.warn("Component " + getComponentName() + " is expired on path " + id); } + if (reset) { + this.timeEnter = 0; + } + } return expired; } return false; } - - public long enterTime() { - return TIME_ENTER_UPDATER.get(this); - } } \ No newline at end of file diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java index 0e4d7ec630..ca8e2b8552 100644 --- a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/critical/CriticalMeasureTest.java @@ -28,7 +28,7 @@ public class CriticalMeasureTest { public void testCriticalMeasure() throws Exception { CriticalMeasure measure = new CriticalMeasure(null, 1); long time = System.nanoTime(); - CriticalMeasure.TIME_ENTER_UPDATER.set(measure, time - TimeUnit.SECONDS.toNanos(5)); + measure.timeEnter = time - TimeUnit.SECONDS.toNanos(5); Assert.assertFalse(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false)); } @@ -39,7 +39,7 @@ public class CriticalMeasureTest { CriticalMeasure measure = new CriticalMeasure(component, 1); long time = System.nanoTime(); CriticalMeasure.CURRENT_THREAD_UDPATER.set(measure, Thread.currentThread()); - CriticalMeasure.TIME_ENTER_UPDATER.set(measure, time - TimeUnit.MINUTES.toNanos(30)); + measure.timeEnter = time - TimeUnit.MINUTES.toNanos(30); measure.leaveCritical(); Assert.assertFalse(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false)); } @@ -51,8 +51,12 @@ public class CriticalMeasureTest { CriticalMeasure measure = new CriticalMeasure(component, 1); long time = System.nanoTime(); measure.enterCritical(); - CriticalMeasure.TIME_ENTER_UPDATER.set(measure, time - TimeUnit.MINUTES.toNanos(5)); + measure.timeEnter = time - TimeUnit.MINUTES.toNanos(5); Assert.assertTrue(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false)); + // subsequent call without reset should still fail + Assert.assertTrue(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), true)); + // previous reset should have cleared it + Assert.assertFalse(measure.checkExpiration(TimeUnit.SECONDS.toNanos(30), false)); measure.leaveCritical(); } }