From 3198cba903cc19a463fbc3f5e510a1b1dca74a8c Mon Sep 17 00:00:00 2001 From: Maurice van Veen Date: Fri, 23 Aug 2024 19:17:23 +0200 Subject: [PATCH] Fix `cleanResponses` must handle cleaning up cancelled future (#1218) * Fix `cleanResponses` must handle cleaning up cancelled future If a future would be cancelled outside of `cleanResponses`, calling `future.get()` would throw a `CancellationException`. This exception was not caught, breaking future cleanup. Signed-off-by: Maurice van Veen * Catch Throwable Signed-off-by: Maurice van Veen --------- Signed-off-by: Maurice van Veen --- .../io/nats/client/impl/NatsConnection.java | 2 +- .../io/nats/client/impl/RequestTests.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/nats/client/impl/NatsConnection.java b/src/main/java/io/nats/client/impl/NatsConnection.java index 116ea237c..f0c56cbdc 100644 --- a/src/main/java/io/nats/client/impl/NatsConnection.java +++ b/src/main/java/io/nats/client/impl/NatsConnection.java @@ -1171,7 +1171,7 @@ else if (future.isDone()) { wasInterrupted = true; break; } - catch (ExecutionException ignore) {} + catch (Throwable ignore) {} } if (remove) { diff --git a/src/test/java/io/nats/client/impl/RequestTests.java b/src/test/java/io/nats/client/impl/RequestTests.java index ca8da444c..ec030e617 100644 --- a/src/test/java/io/nats/client/impl/RequestTests.java +++ b/src/test/java/io/nats/client/impl/RequestTests.java @@ -739,4 +739,24 @@ public void testNatsImplAndEmptyStatsCoverage() { assertEquals(0, s.getDroppedCount()); assertEquals(0, s.getReconnects()); } + + @Test + public void testCancelledFutureMustNotErrorOnCleanResponses() throws Exception { + try (NatsTestServer ts = new NatsTestServer(false)) { + Options options = Options.builder() + .server(ts.getURI()) + .noNoResponders() + .requestCleanupInterval(Duration.ofSeconds(10)) + .build(); + NatsConnection nc = (NatsConnection) Nats.connect(options); + + NatsRequestCompletableFuture future = (NatsRequestCompletableFuture) nc.request("request", null); + future.cancelClosing(); + + // Future is already cancelled, collecting it shouldn't result in an exception being thrown. + assertDoesNotThrow(() -> { + nc.cleanResponses(false); + }); + } + } }