diff --git a/src/main/docs/yielding-pauser-deadline-tests.adoc b/src/main/docs/yielding-pauser-deadline-tests.adoc new file mode 100644 index 000000000..743cec2f8 --- /dev/null +++ b/src/main/docs/yielding-pauser-deadline-tests.adoc @@ -0,0 +1,29 @@ += Pauser deadline controls +:sectnums: +:lang: en-GB + +The timed `YieldingPauser.pause` starts its deadline on the first call, including +the busy phase, and checks it after yielding. Expiry uses a strict greater-than +comparison. `reset` starts a new deadline. The production clock remains +`System.nanoTime`; a package-local method permits a controlled clock in tests +without changing the public API or allocating objects on the pause path. + +The previous fixture required a 100 ms deadline to be observed within a narrow +wall-clock range. OS descheduling, GC and exception construction are included in +that measurement, making it unsuitable for checking the pauser's deadline. + +`YieldingPauserTest.pause` checks the exact boundary; `resetStartsAnotherFullDeadline` +checks independent deadlines; `busyCallsStartTheDeadlineBeforeYielding` checks the +busy phase; `timeSpentYieldingCountsTowardsDeadline` checks time within yield. +`productionClockExpires` exercises the real clock and yielding path with an +already-expired limit. These tests retain the timeout contract without requiring +the host scheduler to run the test thread within a particular interval. + +`LongPauser` likewise exposes a package-local clock method for its asynchronous +pause only. The production path still uses `System.nanoTime`, and the pause ends +at its exact deadline. `LongPauserTest.testLongAsyncPauser` checks the instant +before and at that deadline for nanosecond, microsecond, millisecond and second +configuration, including repeated resets. `asyncPauseIsResetOnReset` checks +cancellation before expiry. These replace a timing tolerance and a retry inside +the test; neither host delays nor a reversed time-unit conversion can conceal a +wrong deadline. The synchronous `unpauseStopsPausing` control retains a real worker. diff --git a/src/main/java/net/openhft/chronicle/threads/LongPauser.java b/src/main/java/net/openhft/chronicle/threads/LongPauser.java index 9752f397e..8acb602e2 100644 --- a/src/main/java/net/openhft/chronicle/threads/LongPauser.java +++ b/src/main/java/net/openhft/chronicle/threads/LongPauser.java @@ -91,7 +91,7 @@ public void pause() { */ @Override public void asyncPause() { - pauseUntilNS = System.nanoTime() + pauseTimeNS; + pauseUntilNS = nanoTime() + pauseTimeNS; increasePauseTimeNS(); } @@ -102,7 +102,13 @@ public void asyncPause() { */ @Override public boolean asyncPausing() { - return pauseUntilNS > System.nanoTime(); + return pauseUntilNS > nanoTime(); + } + + // Control async deadlines independently of scheduler delays in LongPauserTest. + // The production clock and exclusive end of the pause interval are unchanged. + long nanoTime() { + return System.nanoTime(); } private void showPauses() { diff --git a/src/main/java/net/openhft/chronicle/threads/YieldingPauser.java b/src/main/java/net/openhft/chronicle/threads/YieldingPauser.java index dcbe19acf..40b2b618a 100644 --- a/src/main/java/net/openhft/chronicle/threads/YieldingPauser.java +++ b/src/main/java/net/openhft/chronicle/threads/YieldingPauser.java @@ -72,18 +72,24 @@ public void pause() { @Override public void pause(long timeout, @NotNull TimeUnit timeUnit) throws TimeoutException { if (timeOutStart == Long.MAX_VALUE) - timeOutStart = System.nanoTime(); + timeOutStart = nanoTime(); ++count; if (count < minBusy) return; yield0(); - if (System.nanoTime() - timeOutStart > timeUnit.toNanos(timeout)) + if (nanoTime() - timeOutStart > timeUnit.toNanos(timeout)) throw new TimeoutException(); checkYieldTime(); } + // A package-local clock seam lets deadline tests distinguish pauser behaviour from + // OS descheduling. The production clock and strict timeout boundary are unchanged. + long nanoTime() { + return System.nanoTime(); + } + /** * Records and accumulates the duration of yielding if any, and resets the start time of yielding. */ diff --git a/src/test/java/net/openhft/chronicle/threads/LongPauserTest.java b/src/test/java/net/openhft/chronicle/threads/LongPauserTest.java index ef008d4b1..24f5be318 100644 --- a/src/test/java/net/openhft/chronicle/threads/LongPauserTest.java +++ b/src/test/java/net/openhft/chronicle/threads/LongPauserTest.java @@ -5,6 +5,8 @@ import net.openhft.chronicle.core.Jvm; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -45,42 +47,43 @@ void unpauseStopsPausing() throws InterruptedException { assertTrue(timeTakenMs < pauseMillis / 5, "Took " + timeTakenMs + " to stop"); } - @Test - void testLongAsyncPauser() { - final LongPauser pauser = new LongPauser(0, 0, 1, 1, TimeUnit.MILLISECONDS); - boolean failedOnce = false; + @ParameterizedTest + @EnumSource(value = TimeUnit.class, names = {"NANOSECONDS", "MICROSECONDS", "MILLISECONDS", "SECONDS"}) + void testLongAsyncPauser(TimeUnit unit) { + final ControlledLongPauser pauser = new ControlledLongPauser(unit); + // The old wall-clock tolerance measured descheduling and accidentally converted + // nanoseconds to the requested unit. Check the actual unit conversion and boundary. for (int i = 0; i < 100; i++) { - try { - pauser.asyncPause(); - testUntilUnpaused(pauser, 1, TimeUnit.MILLISECONDS); - pauser.reset(); - testUntilUnpaused(pauser, 0, TimeUnit.MILLISECONDS); - } catch (AssertionError e) { - if (failedOnce) - throw e; - failedOnce = true; - } + pauser.asyncPause(); + assertTrue(pauser.asyncPausing()); + pauser.now += unit.toNanos(1) - 1; + assertTrue(pauser.asyncPausing()); + pauser.now++; + assertFalse(pauser.asyncPausing()); + pauser.reset(); + assertFalse(pauser.asyncPausing()); } } @Test void asyncPauseIsResetOnReset() { - final LongPauser longPauser = new LongPauser(0, 0, 1, 1, TimeUnit.SECONDS); + final LongPauser longPauser = new ControlledLongPauser(TimeUnit.SECONDS); longPauser.asyncPause(); assertTrue(longPauser.asyncPausing()); longPauser.reset(); assertFalse(longPauser.asyncPausing()); } - private static void testUntilUnpaused(LongPauser pauser, int n, TimeUnit timeUnit) { - long timeNS = timeUnit.convert(n, TimeUnit.NANOSECONDS); - long start = System.nanoTime(); - while (pauser.asyncPausing()) { - if (System.nanoTime() > start + timeNS + 100_000_000) - fail(); + private static final class ControlledLongPauser extends LongPauser { + long now = TimeUnit.SECONDS.toNanos(1); + + ControlledLongPauser(TimeUnit unit) { + super(0, 0, 1, 1, unit); + } + + @Override + long nanoTime() { + return now; } - long time = System.nanoTime() - start; - final int delta = 11_000_000; - assertEquals(timeNS + delta, time, delta); } } diff --git a/src/test/java/net/openhft/chronicle/threads/YieldingPauserTest.java b/src/test/java/net/openhft/chronicle/threads/YieldingPauserTest.java index e68eda44d..fbf8dbb02 100644 --- a/src/test/java/net/openhft/chronicle/threads/YieldingPauserTest.java +++ b/src/test/java/net/openhft/chronicle/threads/YieldingPauserTest.java @@ -3,43 +3,86 @@ */ package net.openhft.chronicle.threads; -import net.openhft.chronicle.core.OS; import org.junit.jupiter.api.Test; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; class YieldingPauserTest extends ThreadsTestCommon { + // Elapsed wall time also includes descheduling, GC and exception construction. + // A controlled clock checks the 100 ms contract exactly, including its strict boundary. @Test - void pause() { - final int pauseTimeMillis = 100; - final YieldingPauser tp = new YieldingPauser(pauseTimeMillis); - for (int i = 0; i < 10; i++) { - final long start = System.currentTimeMillis(); - while (true) { - try { - tp.pause(pauseTimeMillis, TimeUnit.MILLISECONDS); - if (System.currentTimeMillis() - start > 200) - fail(); - } catch (TimeoutException e) { - final long time = System.currentTimeMillis() - start; - // delta used to be 5 for Linux but occasionally we see it blow in Continuous Integration - // a delta of 20 was used here, however in some situations in CI that was not sufficient: - // org.opentest4j.AssertionFailedError: expected: <100.0> but was: <126.0> - int delta = 30; - // macOS CI has taken 190 ms to observe the timeout; retain the existing lower bound. - final int maxTimeMillis = OS.isMacOSX() ? 200 : pauseTimeMillis + delta; - assertTrue(time >= pauseTimeMillis - delta && time <= maxTimeMillis, - () -> "Expected " + (pauseTimeMillis - delta) + " to " + maxTimeMillis - + " ms but was " + time + " ms"); - tp.reset(); - break; - } - } + void pause() throws TimeoutException { + ControlledPauser pauser = new ControlledPauser(0); + pauser.pause(100, TimeUnit.MILLISECONDS); + pauser.now += TimeUnit.MILLISECONDS.toNanos(100); + pauser.pause(100, TimeUnit.MILLISECONDS); + pauser.now++; + assertThrows(TimeoutException.class, () -> pauser.pause(100, TimeUnit.MILLISECONDS)); + } + + @Test + void resetStartsAnotherFullDeadline() throws TimeoutException { + ControlledPauser pauser = new ControlledPauser(0); + pauser.pause(100, TimeUnit.MILLISECONDS); + pauser.now += TimeUnit.MILLISECONDS.toNanos(100) + 1; + assertThrows(TimeoutException.class, () -> pauser.pause(100, TimeUnit.MILLISECONDS)); + pauser.reset(); + pauser.pause(100, TimeUnit.MILLISECONDS); + pauser.now += TimeUnit.MILLISECONDS.toNanos(100); + pauser.pause(100, TimeUnit.MILLISECONDS); + pauser.now++; + assertThrows(TimeoutException.class, () -> pauser.pause(100, TimeUnit.MILLISECONDS)); + } + + @Test + void busyCallsStartTheDeadlineBeforeYielding() throws TimeoutException { + ControlledPauser pauser = new ControlledPauser(3); + pauser.pause(100, TimeUnit.MILLISECONDS); + pauser.now += TimeUnit.MILLISECONDS.toNanos(100) + 1; + pauser.pause(100, TimeUnit.MILLISECONDS); + assertEquals(0, pauser.yields); + assertThrows(TimeoutException.class, () -> pauser.pause(100, TimeUnit.MILLISECONDS)); + assertEquals(1, pauser.yields); + } + + @Test + void timeSpentYieldingCountsTowardsDeadline() { + ControlledPauser pauser = new ControlledPauser(0); + pauser.yieldNanos = TimeUnit.MILLISECONDS.toNanos(100) + 1; + assertThrows(TimeoutException.class, () -> pauser.pause(100, TimeUnit.MILLISECONDS)); + assertEquals(1, pauser.yields); + } + + @Test + void productionClockExpires() { + YieldingPauser pauser = new YieldingPauser(0); + // A negative limit must expire on the first yielding call for a monotonic clock. + // This covers the production clock implementation without a scheduler deadline. + assertThrows(TimeoutException.class, () -> pauser.pause(-1, TimeUnit.NANOSECONDS)); + } + + private static final class ControlledPauser extends YieldingPauser { + private long now = TimeUnit.SECONDS.toNanos(1); + private long yieldNanos; + private int yields; + + ControlledPauser(int minBusy) { + super(minBusy); + } + + @Override + long nanoTime() { + return now; + } + + @Override + void yield0() { + yields++; + now += yieldNanos; } } }