From af7537d0b85d10ca60598496de8dadc7410d710d Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 26 Sep 2023 15:32:24 +0900 Subject: [PATCH 01/22] minimal impl --- .../internal/common/CancellationScheduler.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index a0e51bb1797..fbb09167881 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -62,6 +62,9 @@ public final class CancellationScheduler { private static final AtomicLongFieldUpdater pendingTimeoutNanosUpdater = AtomicLongFieldUpdater.newUpdater(CancellationScheduler.class, "pendingTimeoutNanos"); + private static final AtomicLongFieldUpdater initializeTriggeredUpdater = + AtomicLongFieldUpdater.newUpdater(CancellationScheduler.class, "initializeTriggered"); + private static final Runnable noopPendingTask = () -> { }; @@ -86,6 +89,7 @@ public final class CancellationScheduler { private volatile TimeoutFuture whenTimedOut; @SuppressWarnings("FieldMayBeFinal") private volatile long pendingTimeoutNanos; + private volatile long initializeTriggered; private boolean server; @Nullable private Throwable cause; @@ -99,6 +103,10 @@ public CancellationScheduler(long timeoutNanos) { * Initializes this {@link CancellationScheduler}. */ public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + if (!initializeTriggeredUpdater.compareAndSet(this, 0, 1)) { + // already cancelled or initialized + return; + } if (!eventLoop.inEventLoop()) { eventLoop.execute(() -> init0(eventLoop, task, timeoutNanos, server)); } else { @@ -301,6 +309,15 @@ public void finishNow(@Nullable Throwable cause) { if (isFinishing()) { return; } + + if (initializeTriggeredUpdater.compareAndSet(this, 0, 1)) { + // finish immediately since init hasn't been called yet + state = State.FINISHED; + this.cause = cause; + ((CancellationFuture) whenCancelled()).doComplete(cause); + return; + } + if (isInitialized()) { if (eventLoop.inEventLoop()) { finishNow0(cause); From afcd9d2fb0ddd556f74d582cd748a4ac878496b9 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 26 Sep 2023 15:50:41 +0900 Subject: [PATCH 02/22] add tests --- .../common/CancellationSchedulerTest.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index 21c7eb69fbe..41615d1bfa7 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -28,11 +28,14 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.stream.Stream; import org.assertj.core.data.Offset; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import com.linecorp.armeria.client.ResponseTimeoutException; import com.linecorp.armeria.common.CommonPools; @@ -57,6 +60,21 @@ public boolean canSchedule() { public void run(Throwable cause) {} }; + static final class StatefulCancellationTask implements CancellationTask { + + AtomicReference thrownRef = new AtomicReference<>(); + + @Override + public boolean canSchedule() { + return true; + } + + @Override + public void run(Throwable cause) { + thrownRef.set(cause); + } + } + private static void executeInEventLoop(long initTimeoutNanos, Consumer task) { final AtomicBoolean completed = new AtomicBoolean(); eventExecutor.execute(() -> { @@ -441,6 +459,44 @@ void multiple_ClearTimeoutInWhenCancelling() { await().untilTrue(completed); } + @Test + void immediateFinishTriggersCompletion() { + final CancellationScheduler scheduler = new CancellationScheduler(0); + final Throwable throwable = new Throwable(); + + assertThat(scheduler.whenCancelling()).isNotCompleted(); + assertThat(scheduler.state()).isEqualTo(State.INIT); + + scheduler.finishNow(throwable); + + assertThat(scheduler.whenCancelling()).isNotDone(); + assertThat(scheduler.whenCancelled()).isCompleted(); + assertThat(scheduler.state()).isEqualTo(State.FINISHED); + assertThat(scheduler.cause()).isSameAs(throwable); + } + + @Test + void finishAfterInitNotComplete() { + final CancellationScheduler scheduler = new CancellationScheduler(0); + final Throwable throwable = new Throwable(); + + assertThat(scheduler.whenCancelling()).isNotCompleted(); + assertThat(scheduler.state()).isEqualTo(State.INIT); + + final StatefulCancellationTask task = new StatefulCancellationTask(); + scheduler.init(eventExecutor, task, 0, false); + scheduler.finishNow(throwable); + + await().untilAsserted(() -> assertThat(scheduler.whenCancelled()).isDone()); + + assertThat(scheduler.whenCancelled()).isCompleted(); + assertThat(scheduler.state()).isEqualTo(State.FINISHED); + + // verify the task was actually executed + assertThat(scheduler.whenCancelling()).isCompleted(); + assertThat(task.thrownRef.get()).isSameAs(throwable); + } + static void assertTimeoutWithTolerance(long actualNanos, long expectedNanos) { assertTimeoutWithTolerance(actualNanos, expectedNanos, MILLISECONDS.toNanos(200)); } From 2d052cc60e33258c569b48cb41adaa5c6a663a44 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 27 Sep 2023 15:53:21 +0900 Subject: [PATCH 03/22] Revert "add tests" This reverts commit afcd9d2fb0ddd556f74d582cd748a4ac878496b9. --- .../common/CancellationSchedulerTest.java | 56 ------------------- 1 file changed, 56 deletions(-) diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index 41615d1bfa7..21c7eb69fbe 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -28,14 +28,11 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; -import java.util.stream.Stream; import org.assertj.core.data.Offset; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; -import org.junit.jupiter.params.provider.ValueSource; import com.linecorp.armeria.client.ResponseTimeoutException; import com.linecorp.armeria.common.CommonPools; @@ -60,21 +57,6 @@ public boolean canSchedule() { public void run(Throwable cause) {} }; - static final class StatefulCancellationTask implements CancellationTask { - - AtomicReference thrownRef = new AtomicReference<>(); - - @Override - public boolean canSchedule() { - return true; - } - - @Override - public void run(Throwable cause) { - thrownRef.set(cause); - } - } - private static void executeInEventLoop(long initTimeoutNanos, Consumer task) { final AtomicBoolean completed = new AtomicBoolean(); eventExecutor.execute(() -> { @@ -459,44 +441,6 @@ void multiple_ClearTimeoutInWhenCancelling() { await().untilTrue(completed); } - @Test - void immediateFinishTriggersCompletion() { - final CancellationScheduler scheduler = new CancellationScheduler(0); - final Throwable throwable = new Throwable(); - - assertThat(scheduler.whenCancelling()).isNotCompleted(); - assertThat(scheduler.state()).isEqualTo(State.INIT); - - scheduler.finishNow(throwable); - - assertThat(scheduler.whenCancelling()).isNotDone(); - assertThat(scheduler.whenCancelled()).isCompleted(); - assertThat(scheduler.state()).isEqualTo(State.FINISHED); - assertThat(scheduler.cause()).isSameAs(throwable); - } - - @Test - void finishAfterInitNotComplete() { - final CancellationScheduler scheduler = new CancellationScheduler(0); - final Throwable throwable = new Throwable(); - - assertThat(scheduler.whenCancelling()).isNotCompleted(); - assertThat(scheduler.state()).isEqualTo(State.INIT); - - final StatefulCancellationTask task = new StatefulCancellationTask(); - scheduler.init(eventExecutor, task, 0, false); - scheduler.finishNow(throwable); - - await().untilAsserted(() -> assertThat(scheduler.whenCancelled()).isDone()); - - assertThat(scheduler.whenCancelled()).isCompleted(); - assertThat(scheduler.state()).isEqualTo(State.FINISHED); - - // verify the task was actually executed - assertThat(scheduler.whenCancelling()).isCompleted(); - assertThat(task.thrownRef.get()).isSameAs(throwable); - } - static void assertTimeoutWithTolerance(long actualNanos, long expectedNanos) { assertTimeoutWithTolerance(actualNanos, expectedNanos, MILLISECONDS.toNanos(200)); } From 48666e154c434c93ad8c9013d22cbabd72823745 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 27 Sep 2023 15:53:28 +0900 Subject: [PATCH 04/22] Revert "minimal impl" This reverts commit af7537d0b85d10ca60598496de8dadc7410d710d. --- .../internal/common/CancellationScheduler.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index fbb09167881..a0e51bb1797 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -62,9 +62,6 @@ public final class CancellationScheduler { private static final AtomicLongFieldUpdater pendingTimeoutNanosUpdater = AtomicLongFieldUpdater.newUpdater(CancellationScheduler.class, "pendingTimeoutNanos"); - private static final AtomicLongFieldUpdater initializeTriggeredUpdater = - AtomicLongFieldUpdater.newUpdater(CancellationScheduler.class, "initializeTriggered"); - private static final Runnable noopPendingTask = () -> { }; @@ -89,7 +86,6 @@ public final class CancellationScheduler { private volatile TimeoutFuture whenTimedOut; @SuppressWarnings("FieldMayBeFinal") private volatile long pendingTimeoutNanos; - private volatile long initializeTriggered; private boolean server; @Nullable private Throwable cause; @@ -103,10 +99,6 @@ public CancellationScheduler(long timeoutNanos) { * Initializes this {@link CancellationScheduler}. */ public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { - if (!initializeTriggeredUpdater.compareAndSet(this, 0, 1)) { - // already cancelled or initialized - return; - } if (!eventLoop.inEventLoop()) { eventLoop.execute(() -> init0(eventLoop, task, timeoutNanos, server)); } else { @@ -309,15 +301,6 @@ public void finishNow(@Nullable Throwable cause) { if (isFinishing()) { return; } - - if (initializeTriggeredUpdater.compareAndSet(this, 0, 1)) { - // finish immediately since init hasn't been called yet - state = State.FINISHED; - this.cause = cause; - ((CancellationFuture) whenCancelled()).doComplete(cause); - return; - } - if (isInitialized()) { if (eventLoop.inEventLoop()) { finishNow0(cause); From 0a75a07288f22809289bf596ac650fce442bcc12 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 27 Sep 2023 15:57:59 +0900 Subject: [PATCH 05/22] introduce a start method --- .../common/CancellationScheduler.java | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index a0e51bb1797..5bc28572f06 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -100,22 +100,39 @@ public CancellationScheduler(long timeoutNanos) { */ public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { if (!eventLoop.inEventLoop()) { - eventLoop.execute(() -> init0(eventLoop, task, timeoutNanos, server)); + eventLoop.execute(() -> { + init0(eventLoop, server); + start(task, timeoutNanos); + }); + } else { + init0(eventLoop, server); + start(task, timeoutNanos); + } + } + + public void init(EventExecutor eventLoop, boolean server) { + if (!eventLoop.inEventLoop()) { + eventLoop.execute(() -> init0(eventLoop, server)); } else { - init0(eventLoop, task, timeoutNanos, server); + init0(eventLoop, server); } } - private void init0(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + private void init0(EventExecutor eventLoop, boolean server) { if (state != State.INIT) { return; } this.eventLoop = eventLoop; + this.server = server; + } + + public void start(CancellationTask task, long timeoutNanos) { + assert state == State.INIT; + assert eventLoop != null; this.task = task; if (timeoutNanos > 0) { this.timeoutNanos = timeoutNanos; } - this.server = server; startTimeNanos = System.nanoTime(); if (this.timeoutNanos != 0) { state = State.SCHEDULED; From 56ffff859bd1e6a4071ab506994b4813bdcb09d7 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 27 Sep 2023 16:05:09 +0900 Subject: [PATCH 06/22] asdf --- .../armeria/internal/common/CancellationScheduler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index 5bc28572f06..6e6e8f0c66a 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -127,7 +127,9 @@ private void init0(EventExecutor eventLoop, boolean server) { } public void start(CancellationTask task, long timeoutNanos) { - assert state == State.INIT; + if (state != State.INIT) { + return; + } assert eventLoop != null; this.task = task; if (timeoutNanos > 0) { From 653ba0c2165743910fe3574d47b8634acb4ff534 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 27 Sep 2023 17:23:05 +0900 Subject: [PATCH 07/22] divide init from start --- .../armeria/client/HttpResponseWrapper.java | 5 ++--- .../client/DefaultClientRequestContext.java | 13 ++++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java b/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java index 0795416df6d..aa2e331587e 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java @@ -291,9 +291,8 @@ void initTimeout() { if (ctxExtension != null) { final CancellationScheduler responseCancellationScheduler = ctxExtension.responseCancellationScheduler(); - responseCancellationScheduler.init( - ctx.eventLoop(), newCancellationTask(), - TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis), /* server */ false); + responseCancellationScheduler.start(newCancellationTask(), + TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis)); } } diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index eef72c48c05..f65baa09e0b 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -213,7 +213,6 @@ private DefaultClientRequestContext( requestAutoAbortDelayMillis(options, requestOptions), req, rpcReq, getAttributes(root)); - this.eventLoop = eventLoop; this.options = requireNonNull(options, "options"); this.root = root; @@ -230,6 +229,9 @@ private DefaultClientRequestContext( } else { this.responseCancellationScheduler = responseCancellationScheduler; } + if (eventLoop != null) { + updateEventLoop(eventLoop); + } long writeTimeoutMillis = requestOptions.writeTimeoutMillis(); if (writeTimeoutMillis < 0) { @@ -407,11 +409,16 @@ private void updateEndpoint(@Nullable Endpoint endpoint) { autoFillSchemeAuthorityAndOrigin(); } + private void updateEventLoop(EventLoop eventLoop) { + this.eventLoop = eventLoop; + responseCancellationScheduler.init(eventLoop, false); + } + private void acquireEventLoop(EndpointGroup endpointGroup) { if (eventLoop == null) { final ReleasableHolder releasableEventLoop = options().factory().acquireEventLoop(sessionProtocol(), endpointGroup, endpoint); - eventLoop = releasableEventLoop.get(); + updateEventLoop(releasableEventLoop.get()); log.whenComplete().thenAccept(unused -> releasableEventLoop.release()); } } @@ -497,7 +504,6 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, // So we don't check the nullness of rpcRequest unlike request. // See https://github.com/line/armeria/pull/3251 and https://github.com/line/armeria/issues/3248. - eventLoop = ctx.eventLoop().withoutContext(); options = ctx.options(); root = ctx.root(); @@ -505,6 +511,7 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, log.startRequest(); responseCancellationScheduler = new CancellationScheduler(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis())); + updateEventLoop(ctx.eventLoop().withoutContext()); writeTimeoutMillis = ctx.writeTimeoutMillis(); maxResponseLength = ctx.maxResponseLength(); From d2700a1958f4f77138fe89d87620c05c2ee3887e Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 27 Sep 2023 18:38:38 +0900 Subject: [PATCH 08/22] allow immediate finish --- .../common/CancellationScheduler.java | 36 ++++++++++++---- .../common/CancellationSchedulerTest.java | 43 +++++++++++++++++++ 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index 6e6e8f0c66a..7b158db617d 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -320,7 +320,15 @@ public void finishNow(@Nullable Throwable cause) { if (isFinishing()) { return; } - if (isInitialized()) { + if (!eventLoop.inEventLoop()) { + eventLoop.execute(() -> finishNow(cause)); + return; + } + if (state == State.INIT) { + state = State.FINISHED; + this.cause = getThrowable(server, cause); + ((CancellationFuture) whenCancelled()).doComplete(cause); + } else if (isInitialized()) { if (eventLoop.inEventLoop()) { finishNow0(cause); } else { @@ -482,13 +490,7 @@ private void invokeTask(@Nullable Throwable cause) { return; } - if (cause == null) { - if (server) { - cause = RequestTimeoutException.get(); - } else { - cause = ResponseTimeoutException.get(); - } - } + cause = getThrowable(server, cause); // Set FINISHING to preclude executing other timeout operations from the callbacks of `whenCancelling()` state = State.FINISHING; @@ -506,11 +508,29 @@ private void invokeTask(@Nullable Throwable cause) { ((CancellationFuture) whenCancelled()).doComplete(cause); } + private static Throwable getThrowable(boolean server, @Nullable Throwable cause) { + if (cause != null) { + return cause; + } + if (server) { + cause = RequestTimeoutException.get(); + } else { + cause = ResponseTimeoutException.get(); + } + return cause; + } + @VisibleForTesting State state() { return state; } + @Nullable + @VisibleForTesting + EventExecutor eventLoop() { + return eventLoop; + } + enum State { INIT, INACTIVE, diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index 21c7eb69fbe..50f792d400a 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -33,6 +33,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import com.linecorp.armeria.client.ResponseTimeoutException; import com.linecorp.armeria.common.CommonPools; @@ -441,6 +442,48 @@ void multiple_ClearTimeoutInWhenCancelling() { await().untilTrue(completed); } + @Test + void immediateFinishTriggersCompletion() { + final CancellationScheduler scheduler = new CancellationScheduler(0); + scheduler.init(eventExecutor, true); + await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); + + final Throwable throwable = new Throwable(); + + assertThat(scheduler.whenCancelling()).isNotCompleted(); + assertThat(scheduler.state()).isEqualTo(State.INIT); + + scheduler.finishNow(throwable); + + await().untilAsserted(() -> assertThat(scheduler.state()).isEqualTo(State.FINISHED)); + assertThat(scheduler.whenCancelling()).isNotDone(); + assertThat(scheduler.whenCancelled()).isCompleted(); + assertThat(scheduler.cause()).isSameAs(throwable); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void immediateFinishWithoutCause(boolean server) { + final CancellationScheduler scheduler = new CancellationScheduler(0); + + scheduler.init(eventExecutor, server); + await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); + + assertThat(scheduler.whenCancelling()).isNotCompleted(); + assertThat(scheduler.state()).isEqualTo(State.INIT); + + scheduler.finishNow(); + + await().untilAsserted(() -> assertThat(scheduler.state()).isEqualTo(State.FINISHED)); + assertThat(scheduler.whenCancelling()).isNotDone(); + assertThat(scheduler.whenCancelled()).isCompleted(); + if (server) { + assertThat(scheduler.cause()).isInstanceOf(RequestTimeoutException.class); + } else { + assertThat(scheduler.cause()).isInstanceOf(ResponseTimeoutException.class); + } + } + static void assertTimeoutWithTolerance(long actualNanos, long expectedNanos) { assertTimeoutWithTolerance(actualNanos, expectedNanos, MILLISECONDS.toNanos(200)); } From af8252e94799d36e1b7d24dd1b0b98aa2a4a2a47 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 28 Sep 2023 00:06:31 +0900 Subject: [PATCH 09/22] extract an interface --- .../client/ClientRequestContextBuilder.java | 4 +- .../client/DefaultClientRequestContext.java | 4 +- .../common/CancellationScheduler.java | 551 +----------------- .../common/DefaultCancellationScheduler.java | 546 +++++++++++++++++ .../server/DefaultServiceRequestContext.java | 2 +- .../server/ServiceRequestContextBuilder.java | 4 +- .../DefaultClientRequestContextTest.java | 2 +- .../common/CancellationSchedulerTest.java | 36 +- 8 files changed, 599 insertions(+), 550 deletions(-) create mode 100644 core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index ac80ec33d71..cb250b574f6 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -62,7 +62,7 @@ public void run(Throwable cause) { /* no-op */ } /** * A cancellation scheduler that has been finished. */ - static final CancellationScheduler noopResponseCancellationScheduler = new CancellationScheduler(0); + static final CancellationScheduler noopResponseCancellationScheduler = CancellationScheduler.of(0); static { noopResponseCancellationScheduler @@ -138,7 +138,7 @@ public ClientRequestContext build() { if (timedOut()) { responseCancellationScheduler = noopResponseCancellationScheduler; } else { - responseCancellationScheduler = new CancellationScheduler(0); + responseCancellationScheduler = CancellationScheduler.of(0); final CountDownLatch latch = new CountDownLatch(1); eventLoop().execute(() -> { responseCancellationScheduler.init(eventLoop(), noopCancellationTask, 0, /* server */ false); diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index f65baa09e0b..fbd59b714b1 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -225,7 +225,7 @@ private DefaultClientRequestContext( responseTimeoutMillis = options().responseTimeoutMillis(); } this.responseCancellationScheduler = - new CancellationScheduler(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis)); + CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis)); } else { this.responseCancellationScheduler = responseCancellationScheduler; } @@ -510,7 +510,7 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, log = RequestLog.builder(this); log.startRequest(); responseCancellationScheduler = - new CancellationScheduler(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis())); + CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis())); updateEventLoop(ctx.eventLoop().withoutContext()); writeTimeoutMillis = ctx.writeTimeoutMillis(); maxResponseLength = ctx.maxResponseLength(); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index 7b158db617d..fc6eb6f580b 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 LINE Corporation + * Copyright 2023 LINE Corporation * * LINE Corporation licenses this file to you under the Apache License, * version 2.0 (the "License"); you may not use this file except in compliance @@ -16,533 +16,41 @@ package com.linecorp.armeria.internal.common; -import static com.google.common.base.Preconditions.checkArgument; -import static java.util.concurrent.TimeUnit.NANOSECONDS; - import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.atomic.AtomicLongFieldUpdater; -import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.math.LongMath; -import com.linecorp.armeria.client.ResponseTimeoutException; -import com.linecorp.armeria.common.TimeoutException; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.util.TimeoutMode; -import com.linecorp.armeria.common.util.UnmodifiableFuture; -import com.linecorp.armeria.server.RequestTimeoutException; import io.netty.util.concurrent.EventExecutor; -@SuppressWarnings("UnstableApiUsage") -public final class CancellationScheduler { - - private static final AtomicReferenceFieldUpdater - whenCancellingUpdater = AtomicReferenceFieldUpdater.newUpdater( - CancellationScheduler.class, CancellationFuture.class, "whenCancelling"); - - private static final AtomicReferenceFieldUpdater - whenCancelledUpdater = AtomicReferenceFieldUpdater.newUpdater( - CancellationScheduler.class, CancellationFuture.class, "whenCancelled"); - - private static final AtomicReferenceFieldUpdater - whenTimingOutUpdater = AtomicReferenceFieldUpdater.newUpdater( - CancellationScheduler.class, TimeoutFuture.class, "whenTimingOut"); - - private static final AtomicReferenceFieldUpdater - whenTimedOutUpdater = AtomicReferenceFieldUpdater.newUpdater( - CancellationScheduler.class, TimeoutFuture.class, "whenTimedOut"); - - private static final AtomicReferenceFieldUpdater - pendingTaskUpdater = AtomicReferenceFieldUpdater.newUpdater( - CancellationScheduler.class, Runnable.class, "pendingTask"); - - private static final AtomicLongFieldUpdater pendingTimeoutNanosUpdater = - AtomicLongFieldUpdater.newUpdater(CancellationScheduler.class, "pendingTimeoutNanos"); - - private static final Runnable noopPendingTask = () -> { - }; - - private State state = State.INIT; - private long timeoutNanos; - private long startTimeNanos; - @Nullable - private EventExecutor eventLoop; - @Nullable - private CancellationTask task; - @Nullable - private volatile Runnable pendingTask; - @Nullable - private ScheduledFuture scheduledFuture; - @Nullable - private volatile CancellationFuture whenCancelling; - @Nullable - private volatile CancellationFuture whenCancelled; - @Nullable - private volatile TimeoutFuture whenTimingOut; - @Nullable - private volatile TimeoutFuture whenTimedOut; - @SuppressWarnings("FieldMayBeFinal") - private volatile long pendingTimeoutNanos; - private boolean server; - @Nullable - private Throwable cause; - - public CancellationScheduler(long timeoutNanos) { - this.timeoutNanos = timeoutNanos; - pendingTimeoutNanos = timeoutNanos; - } - - /** - * Initializes this {@link CancellationScheduler}. - */ - public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { - if (!eventLoop.inEventLoop()) { - eventLoop.execute(() -> { - init0(eventLoop, server); - start(task, timeoutNanos); - }); - } else { - init0(eventLoop, server); - start(task, timeoutNanos); - } - } - - public void init(EventExecutor eventLoop, boolean server) { - if (!eventLoop.inEventLoop()) { - eventLoop.execute(() -> init0(eventLoop, server)); - } else { - init0(eventLoop, server); - } - } - - private void init0(EventExecutor eventLoop, boolean server) { - if (state != State.INIT) { - return; - } - this.eventLoop = eventLoop; - this.server = server; - } - - public void start(CancellationTask task, long timeoutNanos) { - if (state != State.INIT) { - return; - } - assert eventLoop != null; - this.task = task; - if (timeoutNanos > 0) { - this.timeoutNanos = timeoutNanos; - } - startTimeNanos = System.nanoTime(); - if (this.timeoutNanos != 0) { - state = State.SCHEDULED; - scheduledFuture = - eventLoop.schedule(() -> invokeTask(null), this.timeoutNanos, NANOSECONDS); - } else { - state = State.INACTIVE; - } - for (;;) { - final Runnable pendingTask = this.pendingTask; - if (pendingTaskUpdater.compareAndSet(this, pendingTask, noopPendingTask)) { - if (pendingTask != null) { - pendingTask.run(); - } - break; - } - } - } - - public void clearTimeout() { - clearTimeout(true); - } - - public void clearTimeout(boolean resetTimeout) { - if (timeoutNanos() == 0) { - return; - } - if (isInitialized()) { - if (eventLoop.inEventLoop()) { - clearTimeout0(resetTimeout); - } else { - eventLoop.execute(() -> clearTimeout0(resetTimeout)); - } - } else { - if (resetTimeout) { - setPendingTimeoutNanos(0); - } - addPendingTask(() -> clearTimeout0(resetTimeout)); - } - } - - private boolean clearTimeout0(boolean resetTimeout) { - assert eventLoop != null && eventLoop.inEventLoop(); - if (state != State.SCHEDULED) { - return true; - } - if (resetTimeout) { - timeoutNanos = 0; - } - assert scheduledFuture != null; - final boolean cancelled = scheduledFuture.cancel(false); - scheduledFuture = null; - if (cancelled) { - state = State.INACTIVE; - } - return cancelled; - } - - public void setTimeoutNanos(TimeoutMode mode, long timeoutNanos) { - switch (mode) { - case SET_FROM_NOW: - setTimeoutNanosFromNow(timeoutNanos); - break; - case SET_FROM_START: - setTimeoutNanosFromStart(timeoutNanos); - break; - case EXTEND: - extendTimeoutNanos(timeoutNanos); - break; - } - } - - private void setTimeoutNanosFromStart(long timeoutNanos) { - checkArgument(timeoutNanos >= 0, "timeoutNanos: %s (expected: >= 0)", timeoutNanos); - if (timeoutNanos == 0) { - clearTimeout(); - return; - } - if (isInitialized()) { - if (eventLoop.inEventLoop()) { - setTimeoutNanosFromStart0(timeoutNanos); - } else { - eventLoop.execute(() -> setTimeoutNanosFromStart0(timeoutNanos)); - } - } else { - setPendingTimeoutNanos(timeoutNanos); - addPendingTask(() -> setTimeoutNanosFromStart0(timeoutNanos)); - } - } - - private void setTimeoutNanosFromStart0(long timeoutNanos) { - assert eventLoop != null && eventLoop.inEventLoop(); - final long passedTimeNanos = System.nanoTime() - startTimeNanos; - final long newTimeoutNanos = LongMath.saturatedSubtract(timeoutNanos, passedTimeNanos); - if (newTimeoutNanos <= 0) { - invokeTask(null); - return; - } - // Cancel the previously scheduled timeout, if exists. - clearTimeout0(true); - this.timeoutNanos = timeoutNanos; - state = State.SCHEDULED; - scheduledFuture = eventLoop.schedule(() -> invokeTask(null), newTimeoutNanos, NANOSECONDS); - } - - private void extendTimeoutNanos(long adjustmentNanos) { - if (adjustmentNanos == 0 || timeoutNanos() == 0) { - return; - } - if (isInitialized()) { - if (eventLoop.inEventLoop()) { - extendTimeoutNanos0(adjustmentNanos); - } else { - eventLoop.execute(() -> extendTimeoutNanos0(adjustmentNanos)); - } - } else { - addPendingTimeoutNanos(adjustmentNanos); - addPendingTask(() -> extendTimeoutNanos0(adjustmentNanos)); - } - } - - private void extendTimeoutNanos0(long adjustmentNanos) { - assert eventLoop != null && eventLoop.inEventLoop() && task != null; - if (state != State.SCHEDULED || !task.canSchedule()) { - return; - } - final long timeoutNanos = this.timeoutNanos; - // Cancel the previously scheduled timeout, if exists. - clearTimeout0(true); - this.timeoutNanos = LongMath.saturatedAdd(timeoutNanos, adjustmentNanos); - if (timeoutNanos <= 0) { - invokeTask(null); - return; - } - state = State.SCHEDULED; - scheduledFuture = eventLoop.schedule(() -> invokeTask(null), this.timeoutNanos, NANOSECONDS); - } - - private void setTimeoutNanosFromNow(long timeoutNanos) { - checkArgument(timeoutNanos > 0, "timeoutNanos: %s (expected: > 0)", timeoutNanos); - if (isInitialized()) { - if (eventLoop.inEventLoop()) { - setTimeoutNanosFromNow0(timeoutNanos); - } else { - final long eventLoopStartTimeNanos = System.nanoTime(); - eventLoop.execute(() -> { - final long passedTimeNanos0 = System.nanoTime() - eventLoopStartTimeNanos; - final long timeoutNanos0 = Math.max(1, timeoutNanos - passedTimeNanos0); - setTimeoutNanosFromNow0(timeoutNanos0); - }); - } - } else { - final long pendingTaskRegisterTimeNanos = System.nanoTime(); - setPendingTimeoutNanos(timeoutNanos); - addPendingTask(() -> { - final long passedTimeNanos0 = System.nanoTime() - pendingTaskRegisterTimeNanos; - final long timeoutNanos0 = Math.max(1, timeoutNanos - passedTimeNanos0); - setTimeoutNanosFromNow0(timeoutNanos0); - }); - } - } - - private void setTimeoutNanosFromNow0(long newTimeoutNanos) { - assert newTimeoutNanos > 0; - assert eventLoop != null && eventLoop.inEventLoop() && task != null; - if (isFinishing() || !task.canSchedule()) { - return; - } - // Cancel the previously scheduled timeout, if exists. - clearTimeout0(true); - final long passedTimeNanos = System.nanoTime() - startTimeNanos; - timeoutNanos = LongMath.saturatedAdd(newTimeoutNanos, passedTimeNanos); - - state = State.SCHEDULED; - scheduledFuture = eventLoop.schedule(() -> invokeTask(null), newTimeoutNanos, NANOSECONDS); - } - - public void finishNow() { - finishNow(null); - } - - public void finishNow(@Nullable Throwable cause) { - if (isFinishing()) { - return; - } - if (!eventLoop.inEventLoop()) { - eventLoop.execute(() -> finishNow(cause)); - return; - } - if (state == State.INIT) { - state = State.FINISHED; - this.cause = getThrowable(server, cause); - ((CancellationFuture) whenCancelled()).doComplete(cause); - } else if (isInitialized()) { - if (eventLoop.inEventLoop()) { - finishNow0(cause); - } else { - eventLoop.execute(() -> finishNow0(cause)); - } - } else { - addPendingTask(() -> finishNow0(cause)); - } - } - - private void finishNow0(@Nullable Throwable cause) { - assert eventLoop != null && eventLoop.inEventLoop() && task != null; - if (isFinishing() || !task.canSchedule()) { - return; - } - if (state == State.SCHEDULED) { - if (clearTimeout0(false)) { - invokeTask(cause); - } - } else { - invokeTask(cause); - } - } - - public boolean isFinished() { - return state == State.FINISHED; - } - - private boolean isFinishing() { - return state == State.FINISHED || state == State.FINISHING; - } +public interface CancellationScheduler { - @Nullable - public Throwable cause() { - return cause; + static CancellationScheduler of(long timeoutNanos) { + return new DefaultCancellationScheduler(timeoutNanos); } - public long timeoutNanos() { - return isInitialized() ? timeoutNanos : pendingTimeoutNanos; - } - - public long startTimeNanos() { - return startTimeNanos; - } - - public CompletableFuture whenCancelling() { - final CancellationFuture whenCancelling = this.whenCancelling; - if (whenCancelling != null) { - return whenCancelling; - } - final CancellationFuture cancellationFuture = new CancellationFuture(); - if (whenCancellingUpdater.compareAndSet(this, null, cancellationFuture)) { - return cancellationFuture; - } else { - return this.whenCancelling; - } - } - - public CompletableFuture whenCancelled() { - final CancellationFuture whenCancelled = this.whenCancelled; - if (whenCancelled != null) { - return whenCancelled; - } - final CancellationFuture cancellationFuture = new CancellationFuture(); - if (whenCancelledUpdater.compareAndSet(this, null, cancellationFuture)) { - return cancellationFuture; - } else { - return this.whenCancelled; - } - } - - @Deprecated - public CompletableFuture whenTimingOut() { - final TimeoutFuture whenTimingOut = this.whenTimingOut; - if (whenTimingOut != null) { - return whenTimingOut; - } - final TimeoutFuture timeoutFuture = new TimeoutFuture(); - if (whenTimingOutUpdater.compareAndSet(this, null, timeoutFuture)) { - whenCancelling().thenAccept(cause -> { - if (cause instanceof TimeoutException) { - timeoutFuture.doComplete(); - } - }); - return timeoutFuture; - } else { - return this.whenTimingOut; - } - } - - @Deprecated - public CompletableFuture whenTimedOut() { - final TimeoutFuture whenTimedOut = this.whenTimedOut; - if (whenTimedOut != null) { - return whenTimedOut; - } - final TimeoutFuture timeoutFuture = new TimeoutFuture(); - if (whenTimedOutUpdater.compareAndSet(this, null, timeoutFuture)) { - whenCancelled().thenAccept(cause -> { - if (cause instanceof TimeoutException) { - timeoutFuture.doComplete(); - } - }); - return timeoutFuture; - } else { - return this.whenTimedOut; - } - } - - @VisibleForTesting - public boolean isInitialized() { - return pendingTask == noopPendingTask && eventLoop != null; - } - - private void addPendingTask(Runnable pendingTask) { - if (!pendingTaskUpdater.compareAndSet(this, null, pendingTask)) { - for (;;) { - final Runnable oldPendingTask = this.pendingTask; - assert oldPendingTask != null; - if (oldPendingTask == noopPendingTask) { - assert eventLoop != null; - eventLoop.execute(pendingTask); - break; - } - final Runnable newPendingTask = () -> { - oldPendingTask.run(); - pendingTask.run(); - }; - if (pendingTaskUpdater.compareAndSet(this, oldPendingTask, newPendingTask)) { - break; - } - } - } - } - - private void setPendingTimeoutNanos(long pendingTimeoutNanos) { - for (;;) { - final long oldPendingTimeoutNanos = this.pendingTimeoutNanos; - if (pendingTimeoutNanosUpdater.compareAndSet(this, oldPendingTimeoutNanos, pendingTimeoutNanos)) { - break; - } - } - } - - private void addPendingTimeoutNanos(long pendingTimeoutNanos) { - for (;;) { - final long oldPendingTimeoutNanos = this.pendingTimeoutNanos; - final long newPendingTimeoutNanos = LongMath.saturatedAdd(oldPendingTimeoutNanos, - pendingTimeoutNanos); - if (pendingTimeoutNanosUpdater.compareAndSet(this, oldPendingTimeoutNanos, - newPendingTimeoutNanos)) { - break; - } - } - } - - private void invokeTask(@Nullable Throwable cause) { - if (task == null) { - return; - } - - cause = getThrowable(server, cause); + void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server); + void init(EventExecutor eventLoop, boolean server); + void start(CancellationTask task, long timeoutNanos); - // Set FINISHING to preclude executing other timeout operations from the callbacks of `whenCancelling()` - state = State.FINISHING; - if (task.canSchedule()) { - ((CancellationFuture) whenCancelling()).doComplete(cause); - } - // Set state first to prevent duplicate execution - state = State.FINISHED; - - // The returned value of `canSchedule()` could've been changed by the callbacks of `whenCancelling()` - if (task.canSchedule()) { - task.run(cause); - } - this.cause = cause; - ((CancellationFuture) whenCancelled()).doComplete(cause); - } - - private static Throwable getThrowable(boolean server, @Nullable Throwable cause) { - if (cause != null) { - return cause; - } - if (server) { - cause = RequestTimeoutException.get(); - } else { - cause = ResponseTimeoutException.get(); - } - return cause; - } - - @VisibleForTesting - State state() { - return state; - } - - @Nullable - @VisibleForTesting - EventExecutor eventLoop() { - return eventLoop; - } - - enum State { - INIT, - INACTIVE, - SCHEDULED, - FINISHING, - FINISHED - } + void setTimeoutNanos(TimeoutMode mode, long timeoutNanos); + boolean isFinished(); + Throwable cause(); + long timeoutNanos(); + long startTimeNanos(); + CompletableFuture whenCancelling(); + CompletableFuture whenCancelled(); + void clearTimeout(); + void clearTimeout(boolean reset); + void finishNow(); + void finishNow(@Nullable Throwable cause); + CompletableFuture whenTimingOut(); + CompletableFuture whenTimedOut(); /** * A cancellation task invoked by the scheduler when its timeout exceeds or invoke by the user. */ - public interface CancellationTask { + interface CancellationTask { /** * Returns {@code true} if the cancellation task can be scheduled. */ @@ -554,16 +62,11 @@ public interface CancellationTask { void run(Throwable cause); } - private static class CancellationFuture extends UnmodifiableFuture { - @Override - protected void doComplete(@Nullable Throwable cause) { - super.doComplete(cause); - } - } - - private static class TimeoutFuture extends UnmodifiableFuture { - void doComplete() { - doComplete(null); - } + enum State { + INIT, + INACTIVE, + SCHEDULED, + FINISHING, + FINISHED } } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java new file mode 100644 index 00000000000..5f2faacf001 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -0,0 +1,546 @@ +/* + * Copyright 2020 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.internal.common; + +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.atomic.AtomicLongFieldUpdater; +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.math.LongMath; + +import com.linecorp.armeria.client.ResponseTimeoutException; +import com.linecorp.armeria.common.TimeoutException; +import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.common.util.TimeoutMode; +import com.linecorp.armeria.common.util.UnmodifiableFuture; +import com.linecorp.armeria.server.RequestTimeoutException; + +import io.netty.util.concurrent.EventExecutor; + +@SuppressWarnings("UnstableApiUsage") +final class DefaultCancellationScheduler implements CancellationScheduler { + + private static final AtomicReferenceFieldUpdater + whenCancellingUpdater = AtomicReferenceFieldUpdater.newUpdater( + DefaultCancellationScheduler.class, CancellationFuture.class, "whenCancelling"); + + private static final AtomicReferenceFieldUpdater + whenCancelledUpdater = AtomicReferenceFieldUpdater.newUpdater( + DefaultCancellationScheduler.class, CancellationFuture.class, "whenCancelled"); + + private static final AtomicReferenceFieldUpdater + whenTimingOutUpdater = AtomicReferenceFieldUpdater.newUpdater( + DefaultCancellationScheduler.class, TimeoutFuture.class, "whenTimingOut"); + + private static final AtomicReferenceFieldUpdater + whenTimedOutUpdater = AtomicReferenceFieldUpdater.newUpdater( + DefaultCancellationScheduler.class, TimeoutFuture.class, "whenTimedOut"); + + private static final AtomicReferenceFieldUpdater + pendingTaskUpdater = AtomicReferenceFieldUpdater.newUpdater( + DefaultCancellationScheduler.class, Runnable.class, "pendingTask"); + + private static final AtomicLongFieldUpdater pendingTimeoutNanosUpdater = + AtomicLongFieldUpdater.newUpdater(DefaultCancellationScheduler.class, "pendingTimeoutNanos"); + + private static final Runnable noopPendingTask = () -> { + }; + + private State state = State.INIT; + private long timeoutNanos; + private long startTimeNanos; + @Nullable + private EventExecutor eventLoop; + @Nullable + private CancellationTask task; + @Nullable + private volatile Runnable pendingTask; + @Nullable + private ScheduledFuture scheduledFuture; + @Nullable + private volatile CancellationFuture whenCancelling; + @Nullable + private volatile CancellationFuture whenCancelled; + @Nullable + private volatile TimeoutFuture whenTimingOut; + @Nullable + private volatile TimeoutFuture whenTimedOut; + @SuppressWarnings("FieldMayBeFinal") + private volatile long pendingTimeoutNanos; + private boolean server; + @Nullable + private Throwable cause; + + public DefaultCancellationScheduler(long timeoutNanos) { + this.timeoutNanos = timeoutNanos; + pendingTimeoutNanos = timeoutNanos; + } + + /** + * Initializes this {@link CancellationScheduler}. + */ + public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + if (!eventLoop.inEventLoop()) { + eventLoop.execute(() -> { + init0(eventLoop, server); + start(task, timeoutNanos); + }); + } else { + init0(eventLoop, server); + start(task, timeoutNanos); + } + } + + public void init(EventExecutor eventLoop, boolean server) { + if (!eventLoop.inEventLoop()) { + eventLoop.execute(() -> init0(eventLoop, server)); + } else { + init0(eventLoop, server); + } + } + + private void init0(EventExecutor eventLoop, boolean server) { + if (state != State.INIT) { + return; + } + this.eventLoop = eventLoop; + this.server = server; + } + + public void start(CancellationTask task, long timeoutNanos) { + if (state != State.INIT) { + return; + } + assert eventLoop != null; + this.task = task; + if (timeoutNanos > 0) { + this.timeoutNanos = timeoutNanos; + } + startTimeNanos = System.nanoTime(); + if (this.timeoutNanos != 0) { + state = State.SCHEDULED; + scheduledFuture = + eventLoop.schedule(() -> invokeTask(null), this.timeoutNanos, NANOSECONDS); + } else { + state = State.INACTIVE; + } + for (;;) { + final Runnable pendingTask = this.pendingTask; + if (pendingTaskUpdater.compareAndSet(this, pendingTask, noopPendingTask)) { + if (pendingTask != null) { + pendingTask.run(); + } + break; + } + } + } + + public void clearTimeout() { + clearTimeout(true); + } + + public void clearTimeout(boolean resetTimeout) { + if (timeoutNanos() == 0) { + return; + } + if (isInitialized()) { + if (eventLoop.inEventLoop()) { + clearTimeout0(resetTimeout); + } else { + eventLoop.execute(() -> clearTimeout0(resetTimeout)); + } + } else { + if (resetTimeout) { + setPendingTimeoutNanos(0); + } + addPendingTask(() -> clearTimeout0(resetTimeout)); + } + } + + private boolean clearTimeout0(boolean resetTimeout) { + assert eventLoop != null && eventLoop.inEventLoop(); + if (state != State.SCHEDULED) { + return true; + } + if (resetTimeout) { + timeoutNanos = 0; + } + assert scheduledFuture != null; + final boolean cancelled = scheduledFuture.cancel(false); + scheduledFuture = null; + if (cancelled) { + state = State.INACTIVE; + } + return cancelled; + } + + public void setTimeoutNanos(TimeoutMode mode, long timeoutNanos) { + switch (mode) { + case SET_FROM_NOW: + setTimeoutNanosFromNow(timeoutNanos); + break; + case SET_FROM_START: + setTimeoutNanosFromStart(timeoutNanos); + break; + case EXTEND: + extendTimeoutNanos(timeoutNanos); + break; + } + } + + private void setTimeoutNanosFromStart(long timeoutNanos) { + checkArgument(timeoutNanos >= 0, "timeoutNanos: %s (expected: >= 0)", timeoutNanos); + if (timeoutNanos == 0) { + clearTimeout(); + return; + } + if (isInitialized()) { + if (eventLoop.inEventLoop()) { + setTimeoutNanosFromStart0(timeoutNanos); + } else { + eventLoop.execute(() -> setTimeoutNanosFromStart0(timeoutNanos)); + } + } else { + setPendingTimeoutNanos(timeoutNanos); + addPendingTask(() -> setTimeoutNanosFromStart0(timeoutNanos)); + } + } + + private void setTimeoutNanosFromStart0(long timeoutNanos) { + assert eventLoop != null && eventLoop.inEventLoop(); + final long passedTimeNanos = System.nanoTime() - startTimeNanos; + final long newTimeoutNanos = LongMath.saturatedSubtract(timeoutNanos, passedTimeNanos); + if (newTimeoutNanos <= 0) { + invokeTask(null); + return; + } + // Cancel the previously scheduled timeout, if exists. + clearTimeout0(true); + this.timeoutNanos = timeoutNanos; + state = State.SCHEDULED; + scheduledFuture = eventLoop.schedule(() -> invokeTask(null), newTimeoutNanos, NANOSECONDS); + } + + private void extendTimeoutNanos(long adjustmentNanos) { + if (adjustmentNanos == 0 || timeoutNanos() == 0) { + return; + } + if (isInitialized()) { + if (eventLoop.inEventLoop()) { + extendTimeoutNanos0(adjustmentNanos); + } else { + eventLoop.execute(() -> extendTimeoutNanos0(adjustmentNanos)); + } + } else { + addPendingTimeoutNanos(adjustmentNanos); + addPendingTask(() -> extendTimeoutNanos0(adjustmentNanos)); + } + } + + private void extendTimeoutNanos0(long adjustmentNanos) { + assert eventLoop != null && eventLoop.inEventLoop() && task != null; + if (state != State.SCHEDULED || !task.canSchedule()) { + return; + } + final long timeoutNanos = this.timeoutNanos; + // Cancel the previously scheduled timeout, if exists. + clearTimeout0(true); + this.timeoutNanos = LongMath.saturatedAdd(timeoutNanos, adjustmentNanos); + if (timeoutNanos <= 0) { + invokeTask(null); + return; + } + state = State.SCHEDULED; + scheduledFuture = eventLoop.schedule(() -> invokeTask(null), this.timeoutNanos, NANOSECONDS); + } + + private void setTimeoutNanosFromNow(long timeoutNanos) { + checkArgument(timeoutNanos > 0, "timeoutNanos: %s (expected: > 0)", timeoutNanos); + if (isInitialized()) { + if (eventLoop.inEventLoop()) { + setTimeoutNanosFromNow0(timeoutNanos); + } else { + final long eventLoopStartTimeNanos = System.nanoTime(); + eventLoop.execute(() -> { + final long passedTimeNanos0 = System.nanoTime() - eventLoopStartTimeNanos; + final long timeoutNanos0 = Math.max(1, timeoutNanos - passedTimeNanos0); + setTimeoutNanosFromNow0(timeoutNanos0); + }); + } + } else { + final long pendingTaskRegisterTimeNanos = System.nanoTime(); + setPendingTimeoutNanos(timeoutNanos); + addPendingTask(() -> { + final long passedTimeNanos0 = System.nanoTime() - pendingTaskRegisterTimeNanos; + final long timeoutNanos0 = Math.max(1, timeoutNanos - passedTimeNanos0); + setTimeoutNanosFromNow0(timeoutNanos0); + }); + } + } + + private void setTimeoutNanosFromNow0(long newTimeoutNanos) { + assert newTimeoutNanos > 0; + assert eventLoop != null && eventLoop.inEventLoop() && task != null; + if (isFinishing() || !task.canSchedule()) { + return; + } + // Cancel the previously scheduled timeout, if exists. + clearTimeout0(true); + final long passedTimeNanos = System.nanoTime() - startTimeNanos; + timeoutNanos = LongMath.saturatedAdd(newTimeoutNanos, passedTimeNanos); + + state = State.SCHEDULED; + scheduledFuture = eventLoop.schedule(() -> invokeTask(null), newTimeoutNanos, NANOSECONDS); + } + + public void finishNow() { + finishNow(null); + } + + public void finishNow(@Nullable Throwable cause) { + if (isFinishing()) { + return; + } + if (!eventLoop.inEventLoop()) { + eventLoop.execute(() -> finishNow(cause)); + return; + } + if (state == State.INIT) { + state = State.FINISHED; + this.cause = getThrowable(server, cause); + ((CancellationFuture) whenCancelled()).doComplete(cause); + } else if (isInitialized()) { + if (eventLoop.inEventLoop()) { + finishNow0(cause); + } else { + eventLoop.execute(() -> finishNow0(cause)); + } + } else { + addPendingTask(() -> finishNow0(cause)); + } + } + + private void finishNow0(@Nullable Throwable cause) { + assert eventLoop != null && eventLoop.inEventLoop() && task != null; + if (isFinishing() || !task.canSchedule()) { + return; + } + if (state == State.SCHEDULED) { + if (clearTimeout0(false)) { + invokeTask(cause); + } + } else { + invokeTask(cause); + } + } + + public boolean isFinished() { + return state == State.FINISHED; + } + + private boolean isFinishing() { + return state == State.FINISHED || state == State.FINISHING; + } + + @Nullable + public Throwable cause() { + return cause; + } + + public long timeoutNanos() { + return isInitialized() ? timeoutNanos : pendingTimeoutNanos; + } + + public long startTimeNanos() { + return startTimeNanos; + } + + public CompletableFuture whenCancelling() { + final CancellationFuture whenCancelling = this.whenCancelling; + if (whenCancelling != null) { + return whenCancelling; + } + final CancellationFuture cancellationFuture = new CancellationFuture(); + if (whenCancellingUpdater.compareAndSet(this, null, cancellationFuture)) { + return cancellationFuture; + } else { + return this.whenCancelling; + } + } + + public CompletableFuture whenCancelled() { + final CancellationFuture whenCancelled = this.whenCancelled; + if (whenCancelled != null) { + return whenCancelled; + } + final CancellationFuture cancellationFuture = new CancellationFuture(); + if (whenCancelledUpdater.compareAndSet(this, null, cancellationFuture)) { + return cancellationFuture; + } else { + return this.whenCancelled; + } + } + + @Deprecated + public CompletableFuture whenTimingOut() { + final TimeoutFuture whenTimingOut = this.whenTimingOut; + if (whenTimingOut != null) { + return whenTimingOut; + } + final TimeoutFuture timeoutFuture = new TimeoutFuture(); + if (whenTimingOutUpdater.compareAndSet(this, null, timeoutFuture)) { + whenCancelling().thenAccept(cause -> { + if (cause instanceof TimeoutException) { + timeoutFuture.doComplete(); + } + }); + return timeoutFuture; + } else { + return this.whenTimingOut; + } + } + + @Deprecated + public CompletableFuture whenTimedOut() { + final TimeoutFuture whenTimedOut = this.whenTimedOut; + if (whenTimedOut != null) { + return whenTimedOut; + } + final TimeoutFuture timeoutFuture = new TimeoutFuture(); + if (whenTimedOutUpdater.compareAndSet(this, null, timeoutFuture)) { + whenCancelled().thenAccept(cause -> { + if (cause instanceof TimeoutException) { + timeoutFuture.doComplete(); + } + }); + return timeoutFuture; + } else { + return this.whenTimedOut; + } + } + + @VisibleForTesting + public boolean isInitialized() { + return pendingTask == noopPendingTask && eventLoop != null; + } + + private void addPendingTask(Runnable pendingTask) { + if (!pendingTaskUpdater.compareAndSet(this, null, pendingTask)) { + for (;;) { + final Runnable oldPendingTask = this.pendingTask; + assert oldPendingTask != null; + if (oldPendingTask == noopPendingTask) { + assert eventLoop != null; + eventLoop.execute(pendingTask); + break; + } + final Runnable newPendingTask = () -> { + oldPendingTask.run(); + pendingTask.run(); + }; + if (pendingTaskUpdater.compareAndSet(this, oldPendingTask, newPendingTask)) { + break; + } + } + } + } + + private void setPendingTimeoutNanos(long pendingTimeoutNanos) { + for (;;) { + final long oldPendingTimeoutNanos = this.pendingTimeoutNanos; + if (pendingTimeoutNanosUpdater.compareAndSet(this, oldPendingTimeoutNanos, pendingTimeoutNanos)) { + break; + } + } + } + + private void addPendingTimeoutNanos(long pendingTimeoutNanos) { + for (;;) { + final long oldPendingTimeoutNanos = this.pendingTimeoutNanos; + final long newPendingTimeoutNanos = LongMath.saturatedAdd(oldPendingTimeoutNanos, + pendingTimeoutNanos); + if (pendingTimeoutNanosUpdater.compareAndSet(this, oldPendingTimeoutNanos, + newPendingTimeoutNanos)) { + break; + } + } + } + + private void invokeTask(@Nullable Throwable cause) { + if (task == null) { + return; + } + + cause = getThrowable(server, cause); + + // Set FINISHING to preclude executing other timeout operations from the callbacks of `whenCancelling()` + state = State.FINISHING; + if (task.canSchedule()) { + ((CancellationFuture) whenCancelling()).doComplete(cause); + } + // Set state first to prevent duplicate execution + state = State.FINISHED; + + // The returned value of `canSchedule()` could've been changed by the callbacks of `whenCancelling()` + if (task.canSchedule()) { + task.run(cause); + } + this.cause = cause; + ((CancellationFuture) whenCancelled()).doComplete(cause); + } + + private static Throwable getThrowable(boolean server, @Nullable Throwable cause) { + if (cause != null) { + return cause; + } + if (server) { + cause = RequestTimeoutException.get(); + } else { + cause = ResponseTimeoutException.get(); + } + return cause; + } + + @VisibleForTesting + State state() { + return state; + } + + @Nullable + @VisibleForTesting + EventExecutor eventLoop() { + return eventLoop; + } + + private static class CancellationFuture extends UnmodifiableFuture { + @Override + protected void doComplete(@Nullable Throwable cause) { + super.doComplete(cause); + } + } + + private static class TimeoutFuture extends UnmodifiableFuture { + void doComplete() { + doComplete(null); + } + } +} diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java index 59513b6f784..e1dec2e545f 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java @@ -174,7 +174,7 @@ public DefaultServiceRequestContext( this.requestCancellationScheduler = requestCancellationScheduler; } else { this.requestCancellationScheduler = - new CancellationScheduler(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis())); + CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis())); } this.sslSession = sslSession; this.proxiedAddresses = requireNonNull(proxiedAddresses, "proxiedAddresses"); diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java index 115c21ec731..a5b938ff84d 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java @@ -84,7 +84,7 @@ public void run(Throwable cause) { /* no-op */ } /** * A cancellation scheduler that has been finished. */ - private static final CancellationScheduler noopRequestCancellationScheduler = new CancellationScheduler(0); + private static final CancellationScheduler noopRequestCancellationScheduler = CancellationScheduler.of(0); static { noopRequestCancellationScheduler.init(ImmediateEventExecutor.INSTANCE, noopCancellationTask, @@ -256,7 +256,7 @@ public ServiceRequestContext build() { if (timedOut()) { requestCancellationScheduler = noopRequestCancellationScheduler; } else { - requestCancellationScheduler = new CancellationScheduler(0); + requestCancellationScheduler = CancellationScheduler.of(0); final CountDownLatch latch = new CountDownLatch(1); eventLoop().execute(() -> { requestCancellationScheduler.init(eventLoop(), noopCancellationTask, 0, /* server */ true); diff --git a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java index ad0caae1917..fe9eba91a93 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java @@ -300,7 +300,7 @@ private static DefaultClientRequestContext newContext(ClientOptions clientOption return new DefaultClientRequestContext( mock(EventLoop.class), NoopMeterRegistry.get(), SessionProtocol.H2C, RequestId.random(), HttpMethod.POST, reqTarget, clientOptions, httpRequest, - null, RequestOptions.of(), new CancellationScheduler(0), System.nanoTime(), + null, RequestOptions.of(), CancellationScheduler.of(0), System.nanoTime(), SystemInfo.currentTimeMicros()); } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index 50f792d400a..45ed5b700e3 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -58,10 +58,10 @@ public boolean canSchedule() { public void run(Throwable cause) {} }; - private static void executeInEventLoop(long initTimeoutNanos, Consumer task) { + private static void executeInEventLoop(long initTimeoutNanos, Consumer task) { final AtomicBoolean completed = new AtomicBoolean(); eventExecutor.execute(() -> { - final CancellationScheduler scheduler = new CancellationScheduler(0); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); scheduler.init(eventExecutor, noopTask, initTimeoutNanos, true); task.accept(scheduler); completed.set(true); @@ -254,7 +254,7 @@ void whenTimingOutAndWhenTimedOut() { final AtomicBoolean completed = new AtomicBoolean(); final AtomicBoolean passed = new AtomicBoolean(); eventExecutor.execute(() -> { - final CancellationScheduler scheduler = new CancellationScheduler(0); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); final CancellationTask task = new CancellationTask() { @Override public boolean canSchedule() { @@ -288,11 +288,11 @@ public void run(Throwable cause) { @Test void whenTimingOutAndWhenTimedOut2() { - final AtomicReference> whenTimingOutRef = new AtomicReference<>(); - final AtomicReference> whenTimedOutRef = new AtomicReference<>(); + final AtomicReference> whenTimingOutRef = new AtomicReference<>(); + final AtomicReference> whenTimedOutRef = new AtomicReference<>(); executeInEventLoop(0, scheduler -> { - final CompletableFuture whenTimingOut = scheduler.whenTimingOut(); - final CompletableFuture whenTimedOut = scheduler.whenTimedOut(); + final CompletableFuture whenTimingOut = scheduler.whenCancelling(); + final CompletableFuture whenTimedOut = scheduler.whenCancelled(); assertThat(whenTimingOut).isNotDone(); assertThat(whenTimedOut).isNotDone(); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); @@ -320,7 +320,7 @@ void whenCancellingAndWhenCancelled(boolean server) { } eventExecutor.execute(() -> { - final CancellationScheduler scheduler = new CancellationScheduler(0); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); final CancellationTask task = new CancellationTask() { @Override public boolean canSchedule() { @@ -358,7 +358,7 @@ public void run(Throwable cause) { @Test void pendingTimeout() { - final CancellationScheduler scheduler = new CancellationScheduler(1000); + final CancellationScheduler scheduler = new DefaultCancellationScheduler(1000); scheduler.setTimeoutNanos(EXTEND, 1000); assertThat(scheduler.timeoutNanos()).isEqualTo(2000); scheduler.setTimeoutNanos(SET_FROM_NOW, 1000); @@ -377,28 +377,28 @@ void pendingTimeout() { void evaluatePendingTimeout() { final AtomicBoolean completed = new AtomicBoolean(); eventExecutor.execute(() -> { - CancellationScheduler scheduler = new CancellationScheduler(MILLISECONDS.toNanos(1000)); + CancellationScheduler scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(1000)); scheduler.init(eventExecutor, noopTask, 0, false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(2000)); - scheduler = new CancellationScheduler(MILLISECONDS.toNanos(1000)); + scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); scheduler.init(eventExecutor, noopTask, 0, false); assertTimeoutWithTolerance(scheduler.timeoutNanos(), MILLISECONDS.toNanos(1000)); - scheduler = new CancellationScheduler(MILLISECONDS.toNanos(1000)); + scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(false); scheduler.init(eventExecutor, noopTask, 0, false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(1000)); - scheduler = new CancellationScheduler(MILLISECONDS.toNanos(1000)); + scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(); scheduler.init(eventExecutor, noopTask, 0, false); assertThat(scheduler.timeoutNanos()).isZero(); - scheduler = new CancellationScheduler(MILLISECONDS.toNanos(1000)); + scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(SET_FROM_START, MILLISECONDS.toNanos(10000)); @@ -412,7 +412,7 @@ void evaluatePendingTimeout() { @Test void initializeOnlyOnce() { final AtomicBoolean completed = new AtomicBoolean(); - final CancellationScheduler scheduler = new CancellationScheduler(0); + final CancellationScheduler scheduler = new DefaultCancellationScheduler(0); eventExecutor.execute(() -> { scheduler.init(eventExecutor, noopTask, MILLISECONDS.toNanos(100), false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); @@ -428,7 +428,7 @@ void initializeOnlyOnce() { @Test void multiple_ClearTimeoutInWhenCancelling() { final AtomicBoolean completed = new AtomicBoolean(); - final CancellationScheduler scheduler = new CancellationScheduler(0); + final CancellationScheduler scheduler = new DefaultCancellationScheduler(0); scheduler.whenCancelling().thenRun(() -> { scheduler.clearTimeout(false); scheduler.clearTimeout(false); @@ -444,7 +444,7 @@ void multiple_ClearTimeoutInWhenCancelling() { @Test void immediateFinishTriggersCompletion() { - final CancellationScheduler scheduler = new CancellationScheduler(0); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); scheduler.init(eventExecutor, true); await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); @@ -464,7 +464,7 @@ void immediateFinishTriggersCompletion() { @ParameterizedTest @ValueSource(booleans = {true, false}) void immediateFinishWithoutCause(boolean server) { - final CancellationScheduler scheduler = new CancellationScheduler(0); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); scheduler.init(eventExecutor, server); await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); From bafb8a2ef59e83655645ba543e25246906343684 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 28 Sep 2023 01:11:32 +0900 Subject: [PATCH 10/22] working version --- .../client/ClientRequestContextBuilder.java | 2 +- .../HttpClientPipelineConfigurator.java | 4 +- .../common/CancellationScheduler.java | 5 + .../common/DefaultCancellationScheduler.java | 23 +++- .../common/NoopCancellationScheduler.java | 116 ++++++++++++++++++ .../server/DefaultServiceRequestContext.java | 1 + .../server/AbstractHttpResponseHandler.java | 3 +- 7 files changed, 146 insertions(+), 8 deletions(-) create mode 100644 core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index cb250b574f6..2d74fc5695a 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -62,7 +62,7 @@ public void run(Throwable cause) { /* no-op */ } /** * A cancellation scheduler that has been finished. */ - static final CancellationScheduler noopResponseCancellationScheduler = CancellationScheduler.of(0); + private static final CancellationScheduler noopResponseCancellationScheduler = CancellationScheduler.of(0); static { noopResponseCancellationScheduler diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientPipelineConfigurator.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientPipelineConfigurator.java index 5009687de82..3c3024911ea 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientPipelineConfigurator.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientPipelineConfigurator.java @@ -17,7 +17,6 @@ package com.linecorp.armeria.client; import static com.google.common.base.MoreObjects.firstNonNull; -import static com.linecorp.armeria.client.ClientRequestContextBuilder.noopResponseCancellationScheduler; import static com.linecorp.armeria.common.SessionProtocol.H1; import static com.linecorp.armeria.common.SessionProtocol.H1C; import static com.linecorp.armeria.common.SessionProtocol.H2; @@ -57,6 +56,7 @@ import com.linecorp.armeria.internal.client.UserAgentUtil; import com.linecorp.armeria.internal.common.ArmeriaHttp2HeadersDecoder; import com.linecorp.armeria.internal.common.ArmeriaHttpUtil; +import com.linecorp.armeria.internal.common.CancellationScheduler; import com.linecorp.armeria.internal.common.ReadSuppressingHandler; import com.linecorp.armeria.internal.common.TrafficLoggingHandler; import com.linecorp.armeria.internal.common.util.ChannelUtil; @@ -542,7 +542,7 @@ public void onComplete() {} com.linecorp.armeria.common.HttpMethod.OPTIONS, RequestTarget.forClient("*"), ClientOptions.of(), HttpRequest.of(com.linecorp.armeria.common.HttpMethod.OPTIONS, "*"), - null, REQUEST_OPTIONS_FOR_UPGRADE_REQUEST, noopResponseCancellationScheduler, + null, REQUEST_OPTIONS_FOR_UPGRADE_REQUEST, CancellationScheduler.noop(), System.nanoTime(), SystemInfo.currentTimeMicros()); // NB: No need to set the response timeout because we have session creation timeout. diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index fc6eb6f580b..945bf27081f 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -29,12 +29,17 @@ static CancellationScheduler of(long timeoutNanos) { return new DefaultCancellationScheduler(timeoutNanos); } + static CancellationScheduler noop() { + return NoopCancellationScheduler.INSTANCE; + } + void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server); void init(EventExecutor eventLoop, boolean server); void start(CancellationTask task, long timeoutNanos); void setTimeoutNanos(TimeoutMode mode, long timeoutNanos); boolean isFinished(); + @Nullable Throwable cause(); long timeoutNanos(); long startTimeNanos(); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 5f2faacf001..fb8a3a4eab4 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -62,6 +62,10 @@ final class DefaultCancellationScheduler implements CancellationScheduler { private static final AtomicLongFieldUpdater pendingTimeoutNanosUpdater = AtomicLongFieldUpdater.newUpdater(DefaultCancellationScheduler.class, "pendingTimeoutNanos"); + private static final AtomicReferenceFieldUpdater + eventLoopUpdater = AtomicReferenceFieldUpdater.newUpdater( + DefaultCancellationScheduler.class, EventExecutor.class, "eventLoop"); + private static final Runnable noopPendingTask = () -> { }; @@ -69,7 +73,7 @@ final class DefaultCancellationScheduler implements CancellationScheduler { private long timeoutNanos; private long startTimeNanos; @Nullable - private EventExecutor eventLoop; + private volatile EventExecutor eventLoop; @Nullable private CancellationTask task; @Nullable @@ -99,6 +103,9 @@ public DefaultCancellationScheduler(long timeoutNanos) { * Initializes this {@link CancellationScheduler}. */ public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { + return; + } if (!eventLoop.inEventLoop()) { eventLoop.execute(() -> { init0(eventLoop, server); @@ -111,6 +118,9 @@ public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNan } public void init(EventExecutor eventLoop, boolean server) { + if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { + return; + } if (!eventLoop.inEventLoop()) { eventLoop.execute(() -> init0(eventLoop, server)); } else { @@ -122,12 +132,19 @@ private void init0(EventExecutor eventLoop, boolean server) { if (state != State.INIT) { return; } - this.eventLoop = eventLoop; this.server = server; } public void start(CancellationTask task, long timeoutNanos) { - if (state != State.INIT) { + assert eventLoop != null; + assert eventLoop.inEventLoop(); + if (isFinished()) { + assert cause != null; + task.run(cause); + } + if (this.task != null) { + // just replace the task, there is already a pending timeout schedule running + this.task = task; return; } assert eventLoop != null; diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java new file mode 100644 index 00000000000..d28e9985ed8 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java @@ -0,0 +1,116 @@ +/* + * Copyright 2023 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.internal.common; + +import java.util.concurrent.CompletableFuture; + +import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.common.util.TimeoutMode; +import com.linecorp.armeria.common.util.UnmodifiableFuture; + +import io.netty.util.concurrent.EventExecutor; + +/** + * A {@link CancellationScheduler} which never times out. + */ +class NoopCancellationScheduler implements CancellationScheduler { + + static final CancellationScheduler INSTANCE = new NoopCancellationScheduler(); + + static final CompletableFuture THROWABLE_FUTURE = UnmodifiableFuture.wrap(new CompletableFuture<>()); + static final CompletableFuture VOID_FUTURE = UnmodifiableFuture.wrap(new CompletableFuture<>()); + + @Override + public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + + } + + @Override + public void init(EventExecutor eventLoop, boolean server) { + + } + + @Override + public void start(CancellationTask task, long timeoutNanos) { + + } + + @Override + public void setTimeoutNanos(TimeoutMode mode, long timeoutNanos) { + + } + + @Override + public boolean isFinished() { + return false; + } + + @Override + public Throwable cause() { + return null; + } + + @Override + public long timeoutNanos() { + return 0; + } + + @Override + public long startTimeNanos() { + return 0; + } + + @Override + public CompletableFuture whenCancelling() { + return THROWABLE_FUTURE; + } + + @Override + public CompletableFuture whenCancelled() { + return THROWABLE_FUTURE; + } + + @Override + public void clearTimeout() { + + } + + @Override + public void clearTimeout(boolean reset) { + + } + + @Override + public void finishNow() { + + } + + @Override + public void finishNow(@Nullable Throwable cause) { + + } + + @Override + public CompletableFuture whenTimingOut() { + return VOID_FUTURE; + } + + @Override + public CompletableFuture whenTimedOut() { + return VOID_FUTURE; + } +} diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java index e1dec2e545f..d5ce5845419 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java @@ -176,6 +176,7 @@ public DefaultServiceRequestContext( this.requestCancellationScheduler = CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis())); } + this.requestCancellationScheduler.init(eventLoop(), true); this.sslSession = sslSession; this.proxiedAddresses = requireNonNull(proxiedAddresses, "proxiedAddresses"); this.clientAddress = requireNonNull(clientAddress, "clientAddress"); diff --git a/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java b/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java index 5f289a8a0ac..d0ff73a2ecb 100644 --- a/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java +++ b/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java @@ -241,8 +241,7 @@ final void maybeWriteAccessLog() { */ final void scheduleTimeout() { // Schedule the initial request timeout with the timeoutNanos in the CancellationScheduler - reqCtx.requestCancellationScheduler().init(reqCtx.eventLoop(), newCancellationTask(), - 0, /* server */ true); + reqCtx.requestCancellationScheduler().start(newCancellationTask(), 0); } /** From 96b5fc1f2376819ebd0fc898f39191f72caa2885 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 28 Sep 2023 01:31:26 +0900 Subject: [PATCH 11/22] lint --- .../internal/common/CancellationScheduler.java | 15 +++++++++++++++ .../common/DefaultCancellationScheduler.java | 18 +++++++++++++++++- .../common/NoopCancellationScheduler.java | 11 ++--------- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index 945bf27081f..2ee972cee04 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -34,24 +34,39 @@ static CancellationScheduler noop() { } void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server); + void init(EventExecutor eventLoop, boolean server); + void start(CancellationTask task, long timeoutNanos); void setTimeoutNanos(TimeoutMode mode, long timeoutNanos); + boolean isFinished(); + @Nullable + Throwable cause(); + long timeoutNanos(); + long startTimeNanos(); + CompletableFuture whenCancelling(); + CompletableFuture whenCancelled(); + void clearTimeout(); + void clearTimeout(boolean reset); + void finishNow(); + void finishNow(@Nullable Throwable cause); CompletableFuture whenTimingOut(); + CompletableFuture whenTimedOut(); + /** * A cancellation task invoked by the scheduler when its timeout exceeds or invoke by the user. */ diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index fb8a3a4eab4..5465a46eca0 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -94,7 +94,7 @@ final class DefaultCancellationScheduler implements CancellationScheduler { @Nullable private Throwable cause; - public DefaultCancellationScheduler(long timeoutNanos) { + DefaultCancellationScheduler(long timeoutNanos) { this.timeoutNanos = timeoutNanos; pendingTimeoutNanos = timeoutNanos; } @@ -102,6 +102,7 @@ public DefaultCancellationScheduler(long timeoutNanos) { /** * Initializes this {@link CancellationScheduler}. */ + @Override public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { return; @@ -117,6 +118,7 @@ public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNan } } + @Override public void init(EventExecutor eventLoop, boolean server) { if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { return; @@ -135,6 +137,7 @@ private void init0(EventExecutor eventLoop, boolean server) { this.server = server; } + @Override public void start(CancellationTask task, long timeoutNanos) { assert eventLoop != null; assert eventLoop.inEventLoop(); @@ -171,10 +174,12 @@ public void start(CancellationTask task, long timeoutNanos) { } } + @Override public void clearTimeout() { clearTimeout(true); } + @Override public void clearTimeout(boolean resetTimeout) { if (timeoutNanos() == 0) { return; @@ -210,6 +215,7 @@ private boolean clearTimeout0(boolean resetTimeout) { return cancelled; } + @Override public void setTimeoutNanos(TimeoutMode mode, long timeoutNanos) { switch (mode) { case SET_FROM_NOW: @@ -329,10 +335,12 @@ private void setTimeoutNanosFromNow0(long newTimeoutNanos) { scheduledFuture = eventLoop.schedule(() -> invokeTask(null), newTimeoutNanos, NANOSECONDS); } + @Override public void finishNow() { finishNow(null); } + @Override public void finishNow(@Nullable Throwable cause) { if (isFinishing()) { return; @@ -370,6 +378,7 @@ private void finishNow0(@Nullable Throwable cause) { } } + @Override public boolean isFinished() { return state == State.FINISHED; } @@ -378,19 +387,23 @@ private boolean isFinishing() { return state == State.FINISHED || state == State.FINISHING; } + @Override @Nullable public Throwable cause() { return cause; } + @Override public long timeoutNanos() { return isInitialized() ? timeoutNanos : pendingTimeoutNanos; } + @Override public long startTimeNanos() { return startTimeNanos; } + @Override public CompletableFuture whenCancelling() { final CancellationFuture whenCancelling = this.whenCancelling; if (whenCancelling != null) { @@ -404,6 +417,7 @@ public CompletableFuture whenCancelling() { } } + @Override public CompletableFuture whenCancelled() { final CancellationFuture whenCancelled = this.whenCancelled; if (whenCancelled != null) { @@ -417,6 +431,7 @@ public CompletableFuture whenCancelled() { } } + @Override @Deprecated public CompletableFuture whenTimingOut() { final TimeoutFuture whenTimingOut = this.whenTimingOut; @@ -436,6 +451,7 @@ public CompletableFuture whenTimingOut() { } } + @Override @Deprecated public CompletableFuture whenTimedOut() { final TimeoutFuture whenTimedOut = this.whenTimedOut; diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java index d28e9985ed8..fee1c50dd09 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java @@ -31,27 +31,24 @@ class NoopCancellationScheduler implements CancellationScheduler { static final CancellationScheduler INSTANCE = new NoopCancellationScheduler(); - static final CompletableFuture THROWABLE_FUTURE = UnmodifiableFuture.wrap(new CompletableFuture<>()); + static final CompletableFuture THROWABLE_FUTURE = + UnmodifiableFuture.wrap(new CompletableFuture<>()); static final CompletableFuture VOID_FUTURE = UnmodifiableFuture.wrap(new CompletableFuture<>()); @Override public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { - } @Override public void init(EventExecutor eventLoop, boolean server) { - } @Override public void start(CancellationTask task, long timeoutNanos) { - } @Override public void setTimeoutNanos(TimeoutMode mode, long timeoutNanos) { - } @Override @@ -86,22 +83,18 @@ public CompletableFuture whenCancelled() { @Override public void clearTimeout() { - } @Override public void clearTimeout(boolean reset) { - } @Override public void finishNow() { - } @Override public void finishNow(@Nullable Throwable cause) { - } @Override From 57e11443193109dfd6a000feab4dc387f7b783da Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 21 Dec 2023 11:47:09 +0900 Subject: [PATCH 12/22] update failing test --- .../client/DefaultClientRequestContext.java | 3 +-- .../common/DefaultCancellationScheduler.java | 24 ++++++++++++------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index 63b8478839c..11cbdefd4eb 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -525,7 +525,6 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, log.startRequest(); responseCancellationScheduler = CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis())); - updateEventLoop(ctx.eventLoop().withoutContext()); writeTimeoutMillis = ctx.writeTimeoutMillis(); maxResponseLength = ctx.maxResponseLength(); @@ -541,7 +540,7 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, // We don't need to acquire an EventLoop for the initial attempt because it's already acquired by // the root context. if (endpoint == null || ctx.endpoint() == endpoint && ctx.log.children().isEmpty()) { - eventLoop = ctx.eventLoop().withoutContext(); + updateEventLoop(ctx.eventLoop().withoutContext()); } else { acquireEventLoop(endpoint); } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 6faa444ada5..953a7195f15 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -32,6 +32,8 @@ import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.util.TimeoutMode; import com.linecorp.armeria.common.util.UnmodifiableFuture; +import com.linecorp.armeria.server.HttpResponseException; +import com.linecorp.armeria.server.HttpStatusException; import com.linecorp.armeria.server.RequestTimeoutException; import io.netty.util.concurrent.EventExecutor; @@ -355,7 +357,7 @@ public void finishNow(@Nullable Throwable cause) { } if (state == State.INIT) { state = State.FINISHED; - this.cause = getThrowable(server, cause); + this.cause = mapThrowable(server, cause); ((CancellationFuture) whenCancelled()).doComplete(cause); } else if (isInitialized()) { if (eventLoop.inEventLoop()) { @@ -527,7 +529,7 @@ private void invokeTask(@Nullable Throwable cause) { return; } - cause = getThrowable(server, cause); + cause = mapThrowable(server, cause); // Set FINISHING to preclude executing other timeout operations from the callbacks of `whenCancelling()` state = State.FINISHING; @@ -545,14 +547,18 @@ private void invokeTask(@Nullable Throwable cause) { ((CancellationFuture) whenCancelled()).doComplete(cause); } - private static Throwable getThrowable(boolean server, @Nullable Throwable cause) { - if (cause != null) { - return cause; + private static Throwable mapThrowable(boolean server, @Nullable Throwable cause) { + if (cause instanceof HttpStatusException || cause instanceof HttpResponseException) { + // Log the requestCause only when an Http{Status,Response}Exception was created with a cause. + cause = cause.getCause(); } - if (server) { - cause = RequestTimeoutException.get(); - } else { - cause = ResponseTimeoutException.get(); + + if (cause == null) { + if (server) { + cause = RequestTimeoutException.get(); + } else { + cause = ResponseTimeoutException.get(); + } } return cause; } From 108df5cf950eeaef66a20d83daf8a8e091238b79 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 26 Dec 2023 15:09:36 +0900 Subject: [PATCH 13/22] rename to initAndStart --- .../client/ClientRequestContextBuilder.java | 3 ++- .../common/CancellationScheduler.java | 2 +- .../common/DefaultCancellationScheduler.java | 5 ++--- .../common/NoopCancellationScheduler.java | 2 +- .../server/ServiceRequestContextBuilder.java | 3 ++- .../common/CancellationSchedulerTest.java | 22 +++++++++---------- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index 6ac8ca5ab1c..bbab1f6ef77 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -119,7 +119,8 @@ public ClientRequestContext build() { responseCancellationScheduler = CancellationScheduler.of(0); final CountDownLatch latch = new CountDownLatch(1); eventLoop().execute(() -> { - responseCancellationScheduler.init(eventLoop(), noopCancellationTask, 0, /* server */ false); + responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, + 0, /* server */ false); latch.countDown(); }); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index 2418d606955..b07988e4cfb 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -59,7 +59,7 @@ public boolean canSchedule() { public void run(Throwable cause) { /* no-op */ } }; - void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server); + void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server); void init(EventExecutor eventLoop, boolean server); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 953a7195f15..8b16b7265db 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -108,8 +108,7 @@ final class DefaultCancellationScheduler implements CancellationScheduler { /** * Initializes this {@link CancellationScheduler}. */ - @Override - public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { return; } @@ -590,7 +589,7 @@ void doComplete() { private static CancellationScheduler finished0(boolean server) { final CancellationScheduler cancellationScheduler = CancellationScheduler.of(0); cancellationScheduler - .init(ImmediateEventExecutor.INSTANCE, noopCancellationTask, 0, server); + .initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask, 0, server); cancellationScheduler.finishNow(); return cancellationScheduler; } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java index f954cf2e2bd..885e6e0f429 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java @@ -37,7 +37,7 @@ private NoopCancellationScheduler() { } @Override - public void init(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { } @Override diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java index d8a7902b792..e2b376c3cde 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java @@ -237,7 +237,8 @@ public ServiceRequestContext build() { requestCancellationScheduler = CancellationScheduler.of(0); final CountDownLatch latch = new CountDownLatch(1); eventLoop().execute(() -> { - requestCancellationScheduler.init(eventLoop(), noopCancellationTask, 0, /* server */ true); + requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, + 0, /* server */ true); latch.countDown(); }); diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index 893c74106cd..6e760bcc279 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -63,7 +63,7 @@ private static void executeInEventLoop(long initTimeoutNanos, final AtomicBoolean completed = new AtomicBoolean(); eventExecutor.execute(() -> { final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); - scheduler.init(eventExecutor, noopTask, initTimeoutNanos, true); + scheduler.initAndStart(eventExecutor, noopTask, initTimeoutNanos, true); task.accept(scheduler); completed.set(true); }); @@ -271,7 +271,7 @@ public void run(Throwable cause) { passed.set(true); } }; - scheduler.init(eventExecutor, task, 0, true); + scheduler.initAndStart(eventExecutor, task, 0, true); assertThat(scheduler.isFinished()).isFalse(); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); @@ -337,7 +337,7 @@ public void run(Throwable cause) { passed.set(true); } }; - scheduler.init(eventExecutor, task, 0, server); + scheduler.initAndStart(eventExecutor, task, 0, server); assertThat(scheduler.isFinished()).isFalse(); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); @@ -380,30 +380,30 @@ void evaluatePendingTimeout() { eventExecutor.execute(() -> { CancellationScheduler scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(1000)); - scheduler.init(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0, false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(2000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); - scheduler.init(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0, false); assertTimeoutWithTolerance(scheduler.timeoutNanos(), MILLISECONDS.toNanos(1000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(false); - scheduler.init(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0, false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(1000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(); - scheduler.init(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0, false); assertThat(scheduler.timeoutNanos()).isZero(); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(SET_FROM_START, MILLISECONDS.toNanos(10000)); - scheduler.init(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0, false); assertTimeoutWithTolerance(scheduler.timeoutNanos(), MILLISECONDS.toNanos(10000)); completed.set(true); }); @@ -415,10 +415,10 @@ void initializeOnlyOnce() { final AtomicBoolean completed = new AtomicBoolean(); final CancellationScheduler scheduler = new DefaultCancellationScheduler(0); eventExecutor.execute(() -> { - scheduler.init(eventExecutor, noopTask, MILLISECONDS.toNanos(100), false); + scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100), false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); - scheduler.init(eventExecutor, noopTask, MILLISECONDS.toNanos(1000), false); + scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(1000), false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); completed.set(true); }); @@ -436,7 +436,7 @@ void multiple_ClearTimeoutInWhenCancelling() { completed.set(true); }); eventExecutor.execute(() -> { - scheduler.init(eventExecutor, noopTask, MILLISECONDS.toNanos(100), false); + scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100), false); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); }); From b60abce6d871e3bad9006f83134fee7bc3fdcfe3 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 26 Dec 2023 15:59:36 +0900 Subject: [PATCH 14/22] server is final --- .../client/ClientRequestContextBuilder.java | 5 +- .../client/DefaultClientRequestContext.java | 6 +-- .../common/CancellationScheduler.java | 8 +-- .../common/DefaultCancellationScheduler.java | 49 +++++++------------ .../common/NoopCancellationScheduler.java | 4 +- .../server/DefaultServiceRequestContext.java | 4 +- .../server/ServiceRequestContextBuilder.java | 5 +- .../DefaultClientRequestContextTest.java | 2 +- .../common/CancellationSchedulerTest.java | 28 +++++------ 9 files changed, 48 insertions(+), 63 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index bbab1f6ef77..ae5d7c0a555 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -116,11 +116,10 @@ public ClientRequestContext build() { if (timedOut()) { responseCancellationScheduler = CancellationScheduler.finished(false); } else { - responseCancellationScheduler = CancellationScheduler.of(0); + responseCancellationScheduler = CancellationScheduler.of(0, false); final CountDownLatch latch = new CountDownLatch(1); eventLoop().execute(() -> { - responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, - 0, /* server */ false); + responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); latch.countDown(); }); diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index 11cbdefd4eb..5a5a2bd7dc3 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -239,7 +239,7 @@ private DefaultClientRequestContext( responseTimeoutMillis = options().responseTimeoutMillis(); } this.responseCancellationScheduler = - CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis)); + CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis), false); } else { this.responseCancellationScheduler = responseCancellationScheduler; } @@ -425,7 +425,7 @@ private void updateEndpoint(@Nullable Endpoint endpoint) { private void updateEventLoop(EventLoop eventLoop) { this.eventLoop = eventLoop; - responseCancellationScheduler.init(eventLoop, false); + responseCancellationScheduler.init(eventLoop); } private void acquireEventLoop(EndpointGroup endpointGroup) { @@ -524,7 +524,7 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, log = RequestLog.builder(this); log.startRequest(); responseCancellationScheduler = - CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis())); + CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis()), false); writeTimeoutMillis = ctx.writeTimeoutMillis(); maxResponseLength = ctx.maxResponseLength(); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index b07988e4cfb..96549b98927 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -27,8 +27,8 @@ public interface CancellationScheduler { - static CancellationScheduler of(long timeoutNanos) { - return new DefaultCancellationScheduler(timeoutNanos); + static CancellationScheduler of(long timeoutNanos, boolean server) { + return new DefaultCancellationScheduler(timeoutNanos, server); } /** @@ -59,9 +59,9 @@ public boolean canSchedule() { public void run(Throwable cause) { /* no-op */ } }; - void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server); + void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos); - void init(EventExecutor eventLoop, boolean server); + void init(EventExecutor eventLoop); void start(CancellationTask task, long timeoutNanos); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 8b16b7265db..3cb6a89303f 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -96,52 +96,43 @@ final class DefaultCancellationScheduler implements CancellationScheduler { private volatile TimeoutFuture whenTimedOut; @SuppressWarnings("FieldMayBeFinal") private volatile long pendingTimeoutNanos; - private boolean server; + private final boolean server; @Nullable private Throwable cause; + @VisibleForTesting DefaultCancellationScheduler(long timeoutNanos) { + this(timeoutNanos, true); + } + + DefaultCancellationScheduler(long timeoutNanos, boolean server) { this.timeoutNanos = timeoutNanos; pendingTimeoutNanos = timeoutNanos; + this.server = server; } /** - * Initializes this {@link CancellationScheduler}. + * Initializes this {@link DefaultCancellationScheduler}. */ - public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + @Override + public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos) { if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { - return; + throw new IllegalStateException("Can't init() more than once"); } if (!eventLoop.inEventLoop()) { - eventLoop.execute(() -> { - init0(eventLoop, server); - start(task, timeoutNanos); - }); + eventLoop.execute(() -> start(task, timeoutNanos)); } else { - init0(eventLoop, server); start(task, timeoutNanos); } } @Override - public void init(EventExecutor eventLoop, boolean server) { + public void init(EventExecutor eventLoop) { if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { - return; - } - if (!eventLoop.inEventLoop()) { - eventLoop.execute(() -> init0(eventLoop, server)); - } else { - init0(eventLoop, server); + throw new IllegalStateException("Can't init() more than once"); } } - private void init0(EventExecutor eventLoop, boolean server) { - if (state != State.INIT) { - return; - } - this.server = server; - } - @Override public void start(CancellationTask task, long timeoutNanos) { assert eventLoop != null; @@ -155,7 +146,6 @@ public void start(CancellationTask task, long timeoutNanos) { this.task = task; return; } - assert eventLoop != null; this.task = task; if (timeoutNanos > 0) { this.timeoutNanos = timeoutNanos; @@ -350,6 +340,7 @@ public void finishNow(@Nullable Throwable cause) { if (isFinishing()) { return; } + assert eventLoop != null; if (!eventLoop.inEventLoop()) { eventLoop.execute(() -> finishNow(cause)); return; @@ -359,11 +350,7 @@ public void finishNow(@Nullable Throwable cause) { this.cause = mapThrowable(server, cause); ((CancellationFuture) whenCancelled()).doComplete(cause); } else if (isInitialized()) { - if (eventLoop.inEventLoop()) { - finishNow0(cause); - } else { - eventLoop.execute(() -> finishNow0(cause)); - } + finishNow0(cause); } else { addPendingTask(() -> finishNow0(cause)); } @@ -587,9 +574,9 @@ void doComplete() { } private static CancellationScheduler finished0(boolean server) { - final CancellationScheduler cancellationScheduler = CancellationScheduler.of(0); + final CancellationScheduler cancellationScheduler = CancellationScheduler.of(0, server); cancellationScheduler - .initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask, 0, server); + .initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask, 0); cancellationScheduler.finishNow(); return cancellationScheduler; } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java index 885e6e0f429..dca5bdb25ef 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java @@ -37,11 +37,11 @@ private NoopCancellationScheduler() { } @Override - public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos, boolean server) { + public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos) { } @Override - public void init(EventExecutor eventLoop, boolean server) { + public void init(EventExecutor eventLoop) { } @Override diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java index baab08fa4d4..4882bd88b0c 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java @@ -177,9 +177,9 @@ public DefaultServiceRequestContext( this.requestCancellationScheduler = requestCancellationScheduler; } else { this.requestCancellationScheduler = - CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis())); + CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis()), true); } - this.requestCancellationScheduler.init(eventLoop(), true); + this.requestCancellationScheduler.init(eventLoop()); this.sslSession = sslSession; this.proxiedAddresses = requireNonNull(proxiedAddresses, "proxiedAddresses"); this.clientAddress = requireNonNull(clientAddress, "clientAddress"); diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java index e2b376c3cde..b86a96da4f7 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java @@ -234,11 +234,10 @@ public ServiceRequestContext build() { if (timedOut()) { requestCancellationScheduler = CancellationScheduler.finished(true); } else { - requestCancellationScheduler = CancellationScheduler.of(0); + requestCancellationScheduler = CancellationScheduler.of(0, true); final CountDownLatch latch = new CountDownLatch(1); eventLoop().execute(() -> { - requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, - 0, /* server */ true); + requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); latch.countDown(); }); diff --git a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java index fe9eba91a93..836401ebdfc 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java @@ -300,7 +300,7 @@ private static DefaultClientRequestContext newContext(ClientOptions clientOption return new DefaultClientRequestContext( mock(EventLoop.class), NoopMeterRegistry.get(), SessionProtocol.H2C, RequestId.random(), HttpMethod.POST, reqTarget, clientOptions, httpRequest, - null, RequestOptions.of(), CancellationScheduler.of(0), System.nanoTime(), + null, RequestOptions.of(), CancellationScheduler.of(0, false), System.nanoTime(), SystemInfo.currentTimeMicros()); } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index 6e760bcc279..fa7aba9c57d 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -63,7 +63,7 @@ private static void executeInEventLoop(long initTimeoutNanos, final AtomicBoolean completed = new AtomicBoolean(); eventExecutor.execute(() -> { final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); - scheduler.initAndStart(eventExecutor, noopTask, initTimeoutNanos, true); + scheduler.initAndStart(eventExecutor, noopTask, initTimeoutNanos); task.accept(scheduler); completed.set(true); }); @@ -271,7 +271,7 @@ public void run(Throwable cause) { passed.set(true); } }; - scheduler.initAndStart(eventExecutor, task, 0, true); + scheduler.initAndStart(eventExecutor, task, 0); assertThat(scheduler.isFinished()).isFalse(); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); @@ -337,7 +337,7 @@ public void run(Throwable cause) { passed.set(true); } }; - scheduler.initAndStart(eventExecutor, task, 0, server); + scheduler.initAndStart(eventExecutor, task, 0); assertThat(scheduler.isFinished()).isFalse(); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); @@ -380,30 +380,30 @@ void evaluatePendingTimeout() { eventExecutor.execute(() -> { CancellationScheduler scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(1000)); - scheduler.initAndStart(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(2000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); - scheduler.initAndStart(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0); assertTimeoutWithTolerance(scheduler.timeoutNanos(), MILLISECONDS.toNanos(1000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(false); - scheduler.initAndStart(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(1000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(); - scheduler.initAndStart(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0); assertThat(scheduler.timeoutNanos()).isZero(); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(SET_FROM_START, MILLISECONDS.toNanos(10000)); - scheduler.initAndStart(eventExecutor, noopTask, 0, false); + scheduler.initAndStart(eventExecutor, noopTask, 0); assertTimeoutWithTolerance(scheduler.timeoutNanos(), MILLISECONDS.toNanos(10000)); completed.set(true); }); @@ -415,10 +415,10 @@ void initializeOnlyOnce() { final AtomicBoolean completed = new AtomicBoolean(); final CancellationScheduler scheduler = new DefaultCancellationScheduler(0); eventExecutor.execute(() -> { - scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100), false); + scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100)); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); - scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(1000), false); + scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(1000)); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); completed.set(true); }); @@ -436,7 +436,7 @@ void multiple_ClearTimeoutInWhenCancelling() { completed.set(true); }); eventExecutor.execute(() -> { - scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100), false); + scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100)); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); }); @@ -446,7 +446,7 @@ void multiple_ClearTimeoutInWhenCancelling() { @Test void immediateFinishTriggersCompletion() { final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); - scheduler.init(eventExecutor, true); + scheduler.init(eventExecutor); await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); final Throwable throwable = new Throwable(); @@ -465,9 +465,9 @@ void immediateFinishTriggersCompletion() { @ParameterizedTest @ValueSource(booleans = {true, false}) void immediateFinishWithoutCause(boolean server) { - final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0, server); - scheduler.init(eventExecutor, server); + scheduler.init(eventExecutor); await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); assertThat(scheduler.whenCancelling()).isNotCompleted(); From 4e50d7c63ad4d89240d349b4e8aee457a16608e7 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 26 Dec 2023 17:32:05 +0900 Subject: [PATCH 15/22] tmp --- .../common/DefaultCancellationScheduler.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 3cb6a89303f..1275c964efb 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -116,9 +116,7 @@ final class DefaultCancellationScheduler implements CancellationScheduler { */ @Override public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos) { - if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { - throw new IllegalStateException("Can't init() more than once"); - } + init(eventLoop); if (!eventLoop.inEventLoop()) { eventLoop.execute(() -> start(task, timeoutNanos)); } else { @@ -345,11 +343,7 @@ public void finishNow(@Nullable Throwable cause) { eventLoop.execute(() -> finishNow(cause)); return; } - if (state == State.INIT) { - state = State.FINISHED; - this.cause = mapThrowable(server, cause); - ((CancellationFuture) whenCancelled()).doComplete(cause); - } else if (isInitialized()) { + if (isInitialized()) { finishNow0(cause); } else { addPendingTask(() -> finishNow0(cause)); @@ -511,9 +505,8 @@ private void addPendingTimeoutNanos(long pendingTimeoutNanos) { } private void invokeTask(@Nullable Throwable cause) { - if (task == null) { - return; - } + assert eventLoop != null && eventLoop.inEventLoop(); + assert state != State.FINISHING && state != State.FINISHED; cause = mapThrowable(server, cause); From 742b40fffb0ee4715057de6e5849e396bcaa2d7c Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 17 Jan 2024 23:43:16 +0900 Subject: [PATCH 16/22] working version --- .../client/ClientRequestContextBuilder.java | 13 +--------- .../client/DefaultClientRequestContext.java | 25 +++++++++---------- .../common/DefaultCancellationScheduler.java | 6 ++++- .../common/NoopCancellationScheduler.java | 2 +- .../server/DefaultServiceRequestContext.java | 2 +- .../server/ServiceRequestContextBuilder.java | 13 +--------- .../common/CancellationSchedulerTest.java | 8 +++--- 7 files changed, 26 insertions(+), 43 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index ae5d7c0a555..c340bc32e28 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -20,8 +20,6 @@ import java.net.InetSocketAddress; import java.net.URI; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLSession; @@ -117,16 +115,7 @@ public ClientRequestContext build() { responseCancellationScheduler = CancellationScheduler.finished(false); } else { responseCancellationScheduler = CancellationScheduler.of(0, false); - final CountDownLatch latch = new CountDownLatch(1); - eventLoop().execute(() -> { - responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); - latch.countDown(); - }); - - try { - latch.await(1000, TimeUnit.MILLISECONDS); - } catch (InterruptedException ignored) { - } + responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); } final DefaultClientRequestContext ctx = new DefaultClientRequestContext( diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index 5a5a2bd7dc3..6f0ab95d59e 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -180,14 +180,15 @@ private static SessionProtocol desiredSessionProtocol(SessionProtocol protocol, * e.g. {@code System.currentTimeMillis() * 1000}. */ public DefaultClientRequestContext( - @Nullable EventLoop eventLoop, MeterRegistry meterRegistry, SessionProtocol sessionProtocol, + EventLoop eventLoop, MeterRegistry meterRegistry, SessionProtocol sessionProtocol, RequestId id, HttpMethod method, RequestTarget reqTarget, ClientOptions options, @Nullable HttpRequest req, @Nullable RpcRequest rpcReq, RequestOptions requestOptions, CancellationScheduler responseCancellationScheduler, long requestStartTimeNanos, long requestStartTimeMicros) { - this(eventLoop, meterRegistry, sessionProtocol, + this(requireNonNull(eventLoop, "eventLoop"), meterRegistry, sessionProtocol, id, method, reqTarget, options, req, rpcReq, requestOptions, serviceRequestContext(), - responseCancellationScheduler, requestStartTimeNanos, requestStartTimeMicros); + requireNonNull(responseCancellationScheduler, "responseCancellationScheduler"), + requestStartTimeNanos, requestStartTimeMicros); } /** @@ -226,7 +227,11 @@ private DefaultClientRequestContext( firstNonNull(requestOptions.exchangeType(), ExchangeType.BIDI_STREAMING), requestAutoAbortDelayMillis(options, requestOptions), req, rpcReq, getAttributes(root), options.contextHook()); + assert (eventLoop == null && responseCancellationScheduler == null) || + (eventLoop != null && responseCancellationScheduler != null) + : "'eventLoop' and 'responseCancellationScheduler' should either be both null or non-null"; + this.eventLoop = eventLoop; this.options = requireNonNull(options, "options"); this.root = root; @@ -243,9 +248,6 @@ private DefaultClientRequestContext( } else { this.responseCancellationScheduler = responseCancellationScheduler; } - if (eventLoop != null) { - updateEventLoop(eventLoop); - } long writeTimeoutMillis = requestOptions.writeTimeoutMillis(); if (writeTimeoutMillis < 0) { @@ -423,17 +425,13 @@ private void updateEndpoint(@Nullable Endpoint endpoint) { autoFillSchemeAuthorityAndOrigin(); } - private void updateEventLoop(EventLoop eventLoop) { - this.eventLoop = eventLoop; - responseCancellationScheduler.init(eventLoop); - } - private void acquireEventLoop(EndpointGroup endpointGroup) { if (eventLoop == null) { final ReleasableHolder releasableEventLoop = options().factory().acquireEventLoop(sessionProtocol(), endpointGroup, endpoint); - updateEventLoop(releasableEventLoop.get()); + eventLoop = releasableEventLoop.get(); log.whenComplete().thenAccept(unused -> releasableEventLoop.release()); + responseCancellationScheduler.init(eventLoop()); } } @@ -540,7 +538,8 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, // We don't need to acquire an EventLoop for the initial attempt because it's already acquired by // the root context. if (endpoint == null || ctx.endpoint() == endpoint && ctx.log.children().isEmpty()) { - updateEventLoop(ctx.eventLoop().withoutContext()); + eventLoop = ctx.eventLoop().withoutContext(); + responseCancellationScheduler.init(ctx.eventLoop()); } else { acquireEventLoop(endpoint); } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 1275c964efb..cfe88c80b52 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -343,7 +343,11 @@ public void finishNow(@Nullable Throwable cause) { eventLoop.execute(() -> finishNow(cause)); return; } - if (isInitialized()) { + if (state == State.INIT) { + state = State.FINISHED; + this.cause = mapThrowable(server, cause); + ((CancellationFuture) whenCancelled()).doComplete(cause); + } else if (isInitialized()) { finishNow0(cause); } else { addPendingTask(() -> finishNow0(cause)); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java index dca5bdb25ef..6fb0d17493d 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java @@ -111,6 +111,6 @@ public CompletableFuture whenTimedOut() { @Override public boolean isInitialized() { - return false; + return true; } } diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java index 4882bd88b0c..9a20c43cb73 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java @@ -178,8 +178,8 @@ public DefaultServiceRequestContext( } else { this.requestCancellationScheduler = CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis()), true); + this.requestCancellationScheduler.init(eventLoop()); } - this.requestCancellationScheduler.init(eventLoop()); this.sslSession = sslSession; this.proxiedAddresses = requireNonNull(proxiedAddresses, "proxiedAddresses"); this.clientAddress = requireNonNull(clientAddress, "clientAddress"); diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java index b86a96da4f7..262c959b4bb 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java @@ -22,8 +22,6 @@ import java.net.InetSocketAddress; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import javax.net.ssl.SSLSession; @@ -235,16 +233,7 @@ public ServiceRequestContext build() { requestCancellationScheduler = CancellationScheduler.finished(true); } else { requestCancellationScheduler = CancellationScheduler.of(0, true); - final CountDownLatch latch = new CountDownLatch(1); - eventLoop().execute(() -> { - requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); - latch.countDown(); - }); - - try { - latch.await(1000, TimeUnit.MILLISECONDS); - } catch (InterruptedException ignored) { - } + requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); } // Build the context with the properties set by a user and the fake objects. diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index fa7aba9c57d..dff5e66d3a4 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -321,7 +321,7 @@ void whenCancellingAndWhenCancelled(boolean server) { } eventExecutor.execute(() -> { - final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0, server); final CancellationTask task = new CancellationTask() { @Override public boolean canSchedule() { @@ -418,8 +418,10 @@ void initializeOnlyOnce() { scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100)); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); - scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(1000)); - assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); + assertThatThrownBy(() -> scheduler.initAndStart(eventExecutor, noopTask, + MILLISECONDS.toNanos(1000))) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Can't init() more than once"); completed.set(true); }); From 11baf7065a1959d96c3cecf149fe8fb3a76b52af Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 18 Jan 2024 00:14:21 +0900 Subject: [PATCH 17/22] minor fix --- .../common/DefaultCancellationScheduler.java | 44 ++++++++----------- .../common/CancellationSchedulerTest.java | 4 +- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index cfe88c80b52..bb3c5e1f90a 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -138,6 +138,7 @@ public void start(CancellationTask task, long timeoutNanos) { if (isFinished()) { assert cause != null; task.run(cause); + return; } if (this.task != null) { // just replace the task, there is already a pending timeout schedule running @@ -343,14 +344,11 @@ public void finishNow(@Nullable Throwable cause) { eventLoop.execute(() -> finishNow(cause)); return; } - if (state == State.INIT) { - state = State.FINISHED; - this.cause = mapThrowable(server, cause); - ((CancellationFuture) whenCancelled()).doComplete(cause); - } else if (isInitialized()) { + if (isInitialized()) { finishNow0(cause); } else { - addPendingTask(() -> finishNow0(cause)); + start(noopCancellationTask, 0); + finishNow0(cause); } } @@ -509,10 +507,22 @@ private void addPendingTimeoutNanos(long pendingTimeoutNanos) { } private void invokeTask(@Nullable Throwable cause) { - assert eventLoop != null && eventLoop.inEventLoop(); - assert state != State.FINISHING && state != State.FINISHED; + if (task == null) { + return; + } - cause = mapThrowable(server, cause); + if (cause instanceof HttpStatusException || cause instanceof HttpResponseException) { + // Log the requestCause only when an Http{Status,Response}Exception was created with a cause. + cause = cause.getCause(); + } + + if (cause == null) { + if (server) { + cause = RequestTimeoutException.get(); + } else { + cause = ResponseTimeoutException.get(); + } + } // Set FINISHING to preclude executing other timeout operations from the callbacks of `whenCancelling()` state = State.FINISHING; @@ -530,22 +540,6 @@ private void invokeTask(@Nullable Throwable cause) { ((CancellationFuture) whenCancelled()).doComplete(cause); } - private static Throwable mapThrowable(boolean server, @Nullable Throwable cause) { - if (cause instanceof HttpStatusException || cause instanceof HttpResponseException) { - // Log the requestCause only when an Http{Status,Response}Exception was created with a cause. - cause = cause.getCause(); - } - - if (cause == null) { - if (server) { - cause = RequestTimeoutException.get(); - } else { - cause = ResponseTimeoutException.get(); - } - } - return cause; - } - @VisibleForTesting State state() { return state; diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index dff5e66d3a4..b86d10d5eb2 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -459,7 +459,7 @@ void immediateFinishTriggersCompletion() { scheduler.finishNow(throwable); await().untilAsserted(() -> assertThat(scheduler.state()).isEqualTo(State.FINISHED)); - assertThat(scheduler.whenCancelling()).isNotDone(); + assertThat(scheduler.whenCancelling()).isCompleted(); assertThat(scheduler.whenCancelled()).isCompleted(); assertThat(scheduler.cause()).isSameAs(throwable); } @@ -478,7 +478,7 @@ void immediateFinishWithoutCause(boolean server) { scheduler.finishNow(); await().untilAsserted(() -> assertThat(scheduler.state()).isEqualTo(State.FINISHED)); - assertThat(scheduler.whenCancelling()).isNotDone(); + assertThat(scheduler.whenCancelling()).isCompleted(); assertThat(scheduler.whenCancelled()).isCompleted(); if (server) { assertThat(scheduler.cause()).isInstanceOf(RequestTimeoutException.class); From e76419219e67d6d0f66f71fe440bb8097ca5808c Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 18 Jan 2024 10:49:46 +0900 Subject: [PATCH 18/22] remove timeout parameter in start method --- .../client/ClientRequestContextBuilder.java | 2 +- .../armeria/client/HttpResponseWrapper.java | 3 +- .../common/CancellationScheduler.java | 4 +-- .../common/DefaultCancellationScheduler.java | 19 +++++------- .../common/NoopCancellationScheduler.java | 4 +-- .../server/AbstractHttpResponseHandler.java | 2 +- .../server/ServiceRequestContextBuilder.java | 2 +- .../common/CancellationSchedulerTest.java | 29 +++++++++---------- 8 files changed, 30 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index c340bc32e28..913fa7f4bc5 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -115,7 +115,7 @@ public ClientRequestContext build() { responseCancellationScheduler = CancellationScheduler.finished(false); } else { responseCancellationScheduler = CancellationScheduler.of(0, false); - responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); + responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask); } final DefaultClientRequestContext ctx = new DefaultClientRequestContext( diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java b/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java index aa2e331587e..a2db00adfa7 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java @@ -291,8 +291,7 @@ void initTimeout() { if (ctxExtension != null) { final CancellationScheduler responseCancellationScheduler = ctxExtension.responseCancellationScheduler(); - responseCancellationScheduler.start(newCancellationTask(), - TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis)); + responseCancellationScheduler.start(newCancellationTask()); } } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index 96549b98927..353af0ad66b 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -59,11 +59,11 @@ public boolean canSchedule() { public void run(Throwable cause) { /* no-op */ } }; - void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos); + void initAndStart(EventExecutor eventLoop, CancellationTask task); void init(EventExecutor eventLoop); - void start(CancellationTask task, long timeoutNanos); + void start(CancellationTask task); void clearTimeout(); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index bb3c5e1f90a..4fa04636da7 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -115,12 +115,12 @@ final class DefaultCancellationScheduler implements CancellationScheduler { * Initializes this {@link DefaultCancellationScheduler}. */ @Override - public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos) { + public void initAndStart(EventExecutor eventLoop, CancellationTask task) { init(eventLoop); if (!eventLoop.inEventLoop()) { - eventLoop.execute(() -> start(task, timeoutNanos)); + eventLoop.execute(() -> start(task)); } else { - start(task, timeoutNanos); + start(task); } } @@ -132,7 +132,7 @@ public void init(EventExecutor eventLoop) { } @Override - public void start(CancellationTask task, long timeoutNanos) { + public void start(CancellationTask task) { assert eventLoop != null; assert eventLoop.inEventLoop(); if (isFinished()) { @@ -146,14 +146,11 @@ public void start(CancellationTask task, long timeoutNanos) { return; } this.task = task; - if (timeoutNanos > 0) { - this.timeoutNanos = timeoutNanos; - } startTimeNanos = System.nanoTime(); - if (this.timeoutNanos != 0) { + if (timeoutNanos != 0) { state = State.SCHEDULED; scheduledFuture = - eventLoop.schedule(() -> invokeTask(null), this.timeoutNanos, NANOSECONDS); + eventLoop.schedule(() -> invokeTask(null), timeoutNanos, NANOSECONDS); } else { state = State.INACTIVE; } @@ -347,7 +344,7 @@ public void finishNow(@Nullable Throwable cause) { if (isInitialized()) { finishNow0(cause); } else { - start(noopCancellationTask, 0); + start(noopCancellationTask); finishNow0(cause); } } @@ -567,7 +564,7 @@ void doComplete() { private static CancellationScheduler finished0(boolean server) { final CancellationScheduler cancellationScheduler = CancellationScheduler.of(0, server); cancellationScheduler - .initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask, 0); + .initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask); cancellationScheduler.finishNow(); return cancellationScheduler; } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java index 6fb0d17493d..24e8e9c3aae 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java @@ -37,7 +37,7 @@ private NoopCancellationScheduler() { } @Override - public void initAndStart(EventExecutor eventLoop, CancellationTask task, long timeoutNanos) { + public void initAndStart(EventExecutor eventLoop, CancellationTask task) { } @Override @@ -45,7 +45,7 @@ public void init(EventExecutor eventLoop) { } @Override - public void start(CancellationTask task, long timeoutNanos) { + public void start(CancellationTask task) { } @Override diff --git a/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java b/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java index ebe91dfa19f..97800c37328 100644 --- a/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java +++ b/core/src/main/java/com/linecorp/armeria/server/AbstractHttpResponseHandler.java @@ -243,7 +243,7 @@ final void maybeWriteAccessLog() { */ final void scheduleTimeout() { // Schedule the initial request timeout with the timeoutNanos in the CancellationScheduler - reqCtx.requestCancellationScheduler().start(newCancellationTask(), 0); + reqCtx.requestCancellationScheduler().start(newCancellationTask()); } /** diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java index 262c959b4bb..10f0257e694 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java @@ -233,7 +233,7 @@ public ServiceRequestContext build() { requestCancellationScheduler = CancellationScheduler.finished(true); } else { requestCancellationScheduler = CancellationScheduler.of(0, true); - requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask, 0); + requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask); } // Build the context with the properties set by a user and the fake objects. diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index b86d10d5eb2..c868d6166ff 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -62,8 +62,8 @@ private static void executeInEventLoop(long initTimeoutNanos, Consumer task) { final AtomicBoolean completed = new AtomicBoolean(); eventExecutor.execute(() -> { - final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); - scheduler.initAndStart(eventExecutor, noopTask, initTimeoutNanos); + final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(initTimeoutNanos); + scheduler.initAndStart(eventExecutor, noopTask); task.accept(scheduler); completed.set(true); }); @@ -271,7 +271,7 @@ public void run(Throwable cause) { passed.set(true); } }; - scheduler.initAndStart(eventExecutor, task, 0); + scheduler.initAndStart(eventExecutor, task); assertThat(scheduler.isFinished()).isFalse(); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); @@ -337,7 +337,7 @@ public void run(Throwable cause) { passed.set(true); } }; - scheduler.initAndStart(eventExecutor, task, 0); + scheduler.initAndStart(eventExecutor, task); assertThat(scheduler.isFinished()).isFalse(); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); @@ -380,30 +380,30 @@ void evaluatePendingTimeout() { eventExecutor.execute(() -> { CancellationScheduler scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(1000)); - scheduler.initAndStart(eventExecutor, noopTask, 0); + scheduler.initAndStart(eventExecutor, noopTask); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(2000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); - scheduler.initAndStart(eventExecutor, noopTask, 0); + scheduler.initAndStart(eventExecutor, noopTask); assertTimeoutWithTolerance(scheduler.timeoutNanos(), MILLISECONDS.toNanos(1000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(false); - scheduler.initAndStart(eventExecutor, noopTask, 0); + scheduler.initAndStart(eventExecutor, noopTask); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(1000)); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.clearTimeout(); - scheduler.initAndStart(eventExecutor, noopTask, 0); + scheduler.initAndStart(eventExecutor, noopTask); assertThat(scheduler.timeoutNanos()).isZero(); scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(EXTEND, MILLISECONDS.toNanos(2000)); scheduler.setTimeoutNanos(SET_FROM_NOW, MILLISECONDS.toNanos(1000)); scheduler.setTimeoutNanos(SET_FROM_START, MILLISECONDS.toNanos(10000)); - scheduler.initAndStart(eventExecutor, noopTask, 0); + scheduler.initAndStart(eventExecutor, noopTask); assertTimeoutWithTolerance(scheduler.timeoutNanos(), MILLISECONDS.toNanos(10000)); completed.set(true); }); @@ -413,13 +413,12 @@ void evaluatePendingTimeout() { @Test void initializeOnlyOnce() { final AtomicBoolean completed = new AtomicBoolean(); - final CancellationScheduler scheduler = new DefaultCancellationScheduler(0); + final CancellationScheduler scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(100)); eventExecutor.execute(() -> { - scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100)); + scheduler.initAndStart(eventExecutor, noopTask); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); - assertThatThrownBy(() -> scheduler.initAndStart(eventExecutor, noopTask, - MILLISECONDS.toNanos(1000))) + assertThatThrownBy(() -> scheduler.initAndStart(eventExecutor, noopTask)) .isInstanceOf(IllegalStateException.class) .hasMessage("Can't init() more than once"); completed.set(true); @@ -431,14 +430,14 @@ void initializeOnlyOnce() { @Test void multiple_ClearTimeoutInWhenCancelling() { final AtomicBoolean completed = new AtomicBoolean(); - final CancellationScheduler scheduler = new DefaultCancellationScheduler(0); + final CancellationScheduler scheduler = new DefaultCancellationScheduler(MILLISECONDS.toNanos(100)); scheduler.whenCancelling().thenRun(() -> { scheduler.clearTimeout(false); scheduler.clearTimeout(false); completed.set(true); }); eventExecutor.execute(() -> { - scheduler.initAndStart(eventExecutor, noopTask, MILLISECONDS.toNanos(100)); + scheduler.initAndStart(eventExecutor, noopTask); assertThat(scheduler.timeoutNanos()).isEqualTo(MILLISECONDS.toNanos(100)); }); From 56890684bc55994dbf866bcaa90d9ea86bf1aff2 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 18 Jan 2024 15:13:12 +0900 Subject: [PATCH 19/22] minor cleanups --- .../client/ClientRequestContextBuilder.java | 2 +- .../client/DefaultClientRequestContext.java | 7 ++++--- .../internal/common/CancellationScheduler.java | 13 ++++++------- .../common/DefaultCancellationScheduler.java | 17 ++++------------- .../common/NoopCancellationScheduler.java | 5 ----- .../server/DefaultServiceRequestContext.java | 2 +- .../server/ServiceRequestContextBuilder.java | 2 +- .../client/DefaultClientRequestContextTest.java | 2 +- .../common/CancellationSchedulerTest.java | 2 -- 9 files changed, 18 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java index 913fa7f4bc5..ca2ae288dbf 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java @@ -114,7 +114,7 @@ public ClientRequestContext build() { if (timedOut()) { responseCancellationScheduler = CancellationScheduler.finished(false); } else { - responseCancellationScheduler = CancellationScheduler.of(0, false); + responseCancellationScheduler = CancellationScheduler.ofClient(0); responseCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask); } diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index 6f0ab95d59e..a66f88c6705 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -229,7 +229,7 @@ private DefaultClientRequestContext( getAttributes(root), options.contextHook()); assert (eventLoop == null && responseCancellationScheduler == null) || (eventLoop != null && responseCancellationScheduler != null) - : "'eventLoop' and 'responseCancellationScheduler' should either be both null or non-null"; + : "'eventLoop' and 'responseCancellationScheduler' should be both null or non-null"; this.eventLoop = eventLoop; this.options = requireNonNull(options, "options"); @@ -244,7 +244,8 @@ private DefaultClientRequestContext( responseTimeoutMillis = options().responseTimeoutMillis(); } this.responseCancellationScheduler = - CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis), false); + CancellationScheduler.ofClient(TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis)); + // the cancellationScheduler is not initialized here since the eventLoop is guaranteed to be null } else { this.responseCancellationScheduler = responseCancellationScheduler; } @@ -522,7 +523,7 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, log = RequestLog.builder(this); log.startRequest(); responseCancellationScheduler = - CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis()), false); + CancellationScheduler.ofClient(TimeUnit.MILLISECONDS.toNanos(ctx.responseTimeoutMillis())); writeTimeoutMillis = ctx.writeTimeoutMillis(); maxResponseLength = ctx.maxResponseLength(); diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java index 353af0ad66b..f8cfe3576b5 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/CancellationScheduler.java @@ -18,8 +18,6 @@ import java.util.concurrent.CompletableFuture; -import com.google.common.annotations.VisibleForTesting; - import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.util.TimeoutMode; @@ -27,8 +25,12 @@ public interface CancellationScheduler { - static CancellationScheduler of(long timeoutNanos, boolean server) { - return new DefaultCancellationScheduler(timeoutNanos, server); + static CancellationScheduler ofClient(long timeoutNanos) { + return new DefaultCancellationScheduler(timeoutNanos, false); + } + + static CancellationScheduler ofServer(long timeoutNanos) { + return new DefaultCancellationScheduler(timeoutNanos, true); } /** @@ -93,9 +95,6 @@ public void run(Throwable cause) { /* no-op */ } @Deprecated CompletableFuture whenTimedOut(); - @VisibleForTesting - boolean isInitialized(); - enum State { INIT, INACTIVE, diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 4fa04636da7..29572d16264 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -39,7 +39,6 @@ import io.netty.util.concurrent.EventExecutor; import io.netty.util.concurrent.ImmediateEventExecutor; -@SuppressWarnings("UnstableApiUsage") final class DefaultCancellationScheduler implements CancellationScheduler { private static final AtomicReferenceFieldUpdater @@ -141,7 +140,7 @@ public void start(CancellationTask task) { return; } if (this.task != null) { - // just replace the task, there is already a pending timeout schedule running + // just replace the task this.task = task; return; } @@ -456,8 +455,7 @@ public CompletableFuture whenTimedOut() { } } - @VisibleForTesting - public boolean isInitialized() { + private boolean isInitialized() { return pendingTask == noopPendingTask && eventLoop != null; } @@ -542,12 +540,6 @@ State state() { return state; } - @Nullable - @VisibleForTesting - EventExecutor eventLoop() { - return eventLoop; - } - private static class CancellationFuture extends UnmodifiableFuture { @Override protected void doComplete(@Nullable Throwable cause) { @@ -562,9 +554,8 @@ void doComplete() { } private static CancellationScheduler finished0(boolean server) { - final CancellationScheduler cancellationScheduler = CancellationScheduler.of(0, server); - cancellationScheduler - .initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask); + final CancellationScheduler cancellationScheduler = new DefaultCancellationScheduler(0, server); + cancellationScheduler.initAndStart(ImmediateEventExecutor.INSTANCE, noopCancellationTask); cancellationScheduler.finishNow(); return cancellationScheduler; } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java index 24e8e9c3aae..bc30d28dd2b 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/NoopCancellationScheduler.java @@ -108,9 +108,4 @@ public CompletableFuture whenTimingOut() { public CompletableFuture whenTimedOut() { return VOID_FUTURE; } - - @Override - public boolean isInitialized() { - return true; - } } diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java index 9a20c43cb73..7753112453c 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java @@ -177,7 +177,7 @@ public DefaultServiceRequestContext( this.requestCancellationScheduler = requestCancellationScheduler; } else { this.requestCancellationScheduler = - CancellationScheduler.of(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis()), true); + CancellationScheduler.ofServer(TimeUnit.MILLISECONDS.toNanos(cfg.requestTimeoutMillis())); this.requestCancellationScheduler.init(eventLoop()); } this.sslSession = sslSession; diff --git a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java index 10f0257e694..d8c5125aefe 100644 --- a/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/server/ServiceRequestContextBuilder.java @@ -232,7 +232,7 @@ public ServiceRequestContext build() { if (timedOut()) { requestCancellationScheduler = CancellationScheduler.finished(true); } else { - requestCancellationScheduler = CancellationScheduler.of(0, true); + requestCancellationScheduler = CancellationScheduler.ofServer(0); requestCancellationScheduler.initAndStart(eventLoop(), noopCancellationTask); } diff --git a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java index 836401ebdfc..a89871937ba 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/client/DefaultClientRequestContextTest.java @@ -300,7 +300,7 @@ private static DefaultClientRequestContext newContext(ClientOptions clientOption return new DefaultClientRequestContext( mock(EventLoop.class), NoopMeterRegistry.get(), SessionProtocol.H2C, RequestId.random(), HttpMethod.POST, reqTarget, clientOptions, httpRequest, - null, RequestOptions.of(), CancellationScheduler.of(0, false), System.nanoTime(), + null, RequestOptions.of(), CancellationScheduler.ofClient(0), System.nanoTime(), SystemInfo.currentTimeMicros()); } diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java index c868d6166ff..380ae29d716 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/CancellationSchedulerTest.java @@ -448,7 +448,6 @@ void multiple_ClearTimeoutInWhenCancelling() { void immediateFinishTriggersCompletion() { final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0); scheduler.init(eventExecutor); - await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); final Throwable throwable = new Throwable(); @@ -469,7 +468,6 @@ void immediateFinishWithoutCause(boolean server) { final DefaultCancellationScheduler scheduler = new DefaultCancellationScheduler(0, server); scheduler.init(eventExecutor); - await().untilAsserted(() -> assertThat(scheduler.eventLoop()).isNotNull()); assertThat(scheduler.whenCancelling()).isNotCompleted(); assertThat(scheduler.state()).isEqualTo(State.INIT); From 6b3849e376924792882889ea9d30bdc22f72423d Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 23 Jan 2024 14:10:11 +0900 Subject: [PATCH 20/22] address comment by @ikhoon --- .../armeria/internal/client/DefaultClientRequestContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index a66f88c6705..c8d961e82f6 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -540,7 +540,7 @@ private DefaultClientRequestContext(DefaultClientRequestContext ctx, // the root context. if (endpoint == null || ctx.endpoint() == endpoint && ctx.log.children().isEmpty()) { eventLoop = ctx.eventLoop().withoutContext(); - responseCancellationScheduler.init(ctx.eventLoop()); + responseCancellationScheduler.init(eventLoop()); } else { acquireEventLoop(endpoint); } From 8750768f4a9e81f399fa8b39a3268f2c47e4375a Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 23 Jan 2024 16:33:31 +0900 Subject: [PATCH 21/22] address comment by @ikhoon --- .../common/DefaultCancellationScheduler.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 29572d16264..2b0d106ee44 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -17,6 +17,7 @@ package com.linecorp.armeria.internal.common; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; import static java.util.concurrent.TimeUnit.NANOSECONDS; import java.util.concurrent.CompletableFuture; @@ -64,10 +65,6 @@ final class DefaultCancellationScheduler implements CancellationScheduler { private static final AtomicLongFieldUpdater pendingTimeoutNanosUpdater = AtomicLongFieldUpdater.newUpdater(DefaultCancellationScheduler.class, "pendingTimeoutNanos"); - private static final AtomicReferenceFieldUpdater - eventLoopUpdater = AtomicReferenceFieldUpdater.newUpdater( - DefaultCancellationScheduler.class, EventExecutor.class, "eventLoop"); - private static final Runnable noopPendingTask = () -> { }; @@ -78,7 +75,7 @@ final class DefaultCancellationScheduler implements CancellationScheduler { private long timeoutNanos; private long startTimeNanos; @Nullable - private volatile EventExecutor eventLoop; + private EventExecutor eventLoop; @Nullable private CancellationTask task; @Nullable @@ -125,9 +122,8 @@ public void initAndStart(EventExecutor eventLoop, CancellationTask task) { @Override public void init(EventExecutor eventLoop) { - if (!eventLoopUpdater.compareAndSet(this, null, eventLoop)) { - throw new IllegalStateException("Can't init() more than once"); - } + checkState(this.eventLoop == null, "Scheduler is already initialized with %s", this.eventLoop); + this.eventLoop = eventLoop; } @Override From e971dd97417a52b4f4172634836658888274a1fe Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 23 Jan 2024 18:13:26 +0900 Subject: [PATCH 22/22] update broken test --- .../armeria/internal/common/DefaultCancellationScheduler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java index 2b0d106ee44..536b3e8a0bc 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultCancellationScheduler.java @@ -122,7 +122,7 @@ public void initAndStart(EventExecutor eventLoop, CancellationTask task) { @Override public void init(EventExecutor eventLoop) { - checkState(this.eventLoop == null, "Scheduler is already initialized with %s", this.eventLoop); + checkState(this.eventLoop == null, "Can't init() more than once"); this.eventLoop = eventLoop; }