From 0b021f71ab5ca26bc56d5813b4f2f83bf5e4ade2 Mon Sep 17 00:00:00 2001 From: Michael Nitschinger Date: Mon, 16 Oct 2023 10:45:09 +0200 Subject: [PATCH] Fix/Debug Flaky CancellationTest Motivation ---------- The CancellationTest#cancelOffload() is reportedly flaky and this PR is a first attempt to understand why / fix the issues. Modifications ------------- Investigation found that it looks like on CI the sleeping of the action races with the cancellation of the service single. Current working theory is that if the cancellation arrives before the service is executed, the thread sleep will never be triggered and so the test completes fine. If the cancellation does not arrive first, the sleep starts to happen and when the cancellation arrives afterwards the thread is interrupted as part of the cancellation which triggers the exception. Current theory is if this is acceptable behavior, the error is harmless and can be ignored. --- .../http/router/jersey/CancellationTest.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/servicetalk-http-router-jersey/src/testFixtures/java/io/servicetalk/http/router/jersey/CancellationTest.java b/servicetalk-http-router-jersey/src/testFixtures/java/io/servicetalk/http/router/jersey/CancellationTest.java index 2618f4db9d..33a22a7a82 100644 --- a/servicetalk-http-router-jersey/src/testFixtures/java/io/servicetalk/http/router/jersey/CancellationTest.java +++ b/servicetalk-http-router-jersey/src/testFixtures/java/io/servicetalk/http/router/jersey/CancellationTest.java @@ -94,7 +94,7 @@ public void cancel() { private static final CharSequence TEST_DATA = newLargePayload(); @RegisterExtension - final ExecutorExtension execRule = ExecutorExtension.withCachedExecutor(); + static final ExecutorExtension execRule = ExecutorExtension.withCachedExecutor().setClassLevel(true); @Mock private HttpServiceContext ctx; @@ -224,8 +224,13 @@ private void testCancelResponseSingle(final StreamingHttpRequest req, jerseyRouter.handle(ctx, req, HTTP_REQ_RES_FACTORY) ).flatMap(identity()) .beforeOnError((err) -> { + // If subscription cancellation arrives after the resource started to sleep, the cancellation + // will interrupt the sleep which is OK. + boolean sleepInterrupted = err instanceof InterruptedException && err.getMessage() + .contains("sleep interrupted"); + // Ignore racy cancellation, it's ordered safely. - if (!(err instanceof IllegalStateException)) { + if (!(err instanceof IllegalStateException || sleepInterrupted)) { errorRef.compareAndSet(null, err); } }) @@ -249,8 +254,13 @@ public void onSuccess(@Nullable final StreamingHttpResponse result) { @Override public void onError(final Throwable t) { + // If subscription cancellation arrives after the resource started to sleep, the cancellation + // will interrupt the sleep which is OK. + boolean sleepInterrupted = t instanceof InterruptedException && t.getMessage() + .contains("sleep interrupted"); + // Ignore racy cancellation, it's ordered safely. - if (!(t instanceof IllegalStateException)) { + if (!(t instanceof IllegalStateException || sleepInterrupted)) { errorRef.compareAndSet(null, t); } cancelledLatch.countDown();