(polaris) branch main updated: NoSQL: Do not interrupt running Vert.x async tasks on cancel (#4437)

[sn...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

snazy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git


The following commit(s) were added to refs/heads/main by this push:
     new 004b58ada2 NoSQL: Do not interrupt running Vert.x async tasks on cancel (#4437)
004b58ada2 is described below

commit 004b58ada2d66ac61cbef1b231f9f8d5ffce43a1
Author: Robert Stupp <sn...@snazy.de>
AuthorDate: Mon May 18 07:57:53 2026 +0200

    NoSQL: Do not interrupt running Vert.x async tasks on cancel (#4437)
    
    Once the task is already running, cancelling the wrapper future should not try to interrupt the Vert.x worker thread.
    
    That was turning a best-effort cancellation into a cross-thread side effect and could kill work that had already legitimately started.
    The added test covers the running-task case explicitly.
    
    In other words: this change aligns the implementation to the documented and intended contract.
---
 .../polaris/nosql/async/AsyncExecTestBase.java     | 31 ++++++++++++++++++++++
 .../polaris/nosql/async/vertx/VertxAsyncExec.java  |  2 +-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/persistence/nosql/async/api/src/testFixtures/java/org/apache/polaris/nosql/async/AsyncExecTestBase.java b/persistence/nosql/async/api/src/testFixtures/java/org/apache/polaris/nosql/async/AsyncExecTestBase.java
index fcc330d04e..be58513145 100644
--- a/persistence/nosql/async/api/src/testFixtures/java/org/apache/polaris/nosql/async/AsyncExecTestBase.java
+++ b/persistence/nosql/async/api/src/testFixtures/java/org/apache/polaris/nosql/async/AsyncExecTestBase.java
@@ -145,6 +145,37 @@ public abstract class AsyncExecTestBase {
         .allSatisfy(cf -> assertThat(cf).succeedsWithin(asyncTimeout).isEqualTo("foo"));
   }
 
+  @Test
+  public void cancelRunningTaskDoesNotInterrupt() throws Exception {
+    var started = new Semaphore(0);
+    var allowFinish = new Semaphore(0);
+    var finished = new Semaphore(0);
+    var interrupted = new AtomicBoolean();
+
+    var cancelable =
+        executor.submit(
+            () -> {
+              started.release();
+              try {
+                allowFinish.acquire();
+              } catch (InterruptedException e) {
+                interrupted.set(true);
+              } finally {
+                finished.release();
+              }
+              return null;
+            });
+
+    soft.assertThat(started.tryAcquire(asyncTimeout.toMillis(), MILLISECONDS)).isTrue();
+
+    cancelable.cancel();
+    allowFinish.release();
+
+    soft.assertThat(finished.tryAcquire(asyncTimeout.toMillis(), MILLISECONDS)).isTrue();
+    soft.assertThat(interrupted.get()).isFalse();
+    cancelledAssert(assertThat(cancelable.completionStage().toCompletableFuture()));
+  }
+
   @Test
   public void submitManyFailing() {
     int numTasks = 50;
diff --git a/persistence/nosql/async/vertx/src/main/java/org/apache/polaris/nosql/async/vertx/VertxAsyncExec.java b/persistence/nosql/async/vertx/src/main/java/org/apache/polaris/nosql/async/vertx/VertxAsyncExec.java
index 4d92cf67a8..3b15a7b7bb 100644
--- a/persistence/nosql/async/vertx/src/main/java/org/apache/polaris/nosql/async/vertx/VertxAsyncExec.java
+++ b/persistence/nosql/async/vertx/src/main/java/org/apache/polaris/nosql/async/vertx/VertxAsyncExec.java
@@ -290,7 +290,7 @@ class VertxAsyncExec implements AsyncExec, AutoCloseable {
       completable.cancel(false);
       var f = runningFuture.getAndSet(null);
       if (f != null && !f.isDone()) {
-        f.cancel(true);
+        f.cancel(false);
       }
       tasks.remove(this);
     }