From bb7c95c09b4fd6d0d8f2830392409c8a2bbbfc75 Mon Sep 17 00:00:00 2001 From: Andy Goryachev Date: Thu, 10 Apr 2025 14:13:10 -0700 Subject: [PATCH 1/3] 8088343 --- .../src/main/java/javafx/concurrent/Task.java | 45 ++++++++++++++----- .../concurrent/ServiceLifecycleTest.java | 26 ++++++----- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java b/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java index e07a200a484..ffde8003ec5 100644 --- a/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java +++ b/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,6 +25,15 @@ package javafx.concurrent; +import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_CANCELLED; +import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_FAILED; +import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_RUNNING; +import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SCHEDULED; +import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SUCCEEDED; +import java.util.concurrent.Callable; +import java.util.concurrent.FutureTask; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import javafx.application.Platform; import javafx.beans.property.BooleanProperty; import javafx.beans.property.DoubleProperty; @@ -43,14 +52,6 @@ import javafx.event.EventHandler; import javafx.event.EventTarget; import javafx.event.EventType; -import java.util.concurrent.Callable; -import java.util.concurrent.FutureTask; -import java.util.concurrent.atomic.AtomicReference; -import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_CANCELLED; -import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_FAILED; -import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_RUNNING; -import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SCHEDULED; -import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SUCCEEDED; /** *

@@ -998,7 +999,8 @@ protected void failed() { } return cancel(true); } - @Override public boolean cancel(boolean mayInterruptIfRunning) { + @Override + public boolean cancel(boolean mayInterruptIfRunning) { // Delegate to the super implementation to actually attempt to cancel this thing // Assert the modifyThread permission boolean flag = super.cancel(mayInterruptIfRunning); @@ -1014,12 +1016,31 @@ protected void failed() { } // state flag will not be readable immediately after this call. However, // that would be the case anyway since these properties are not thread-safe. if (isFxApplicationThread()) { + switch (getState()) { + case FAILED: + case SUCCEEDED: + // a finished or failed task retains its state + return false; + } + setState(Worker.State.CANCELLED); } else { - runLater(() -> setState(Worker.State.CANCELLED)); + AtomicBoolean rv = new AtomicBoolean(flag); + runLater(() -> { + switch (getState()) { + case FAILED: + case SUCCEEDED: + // a finished or failed task retains its state + rv.set(false); + break; + default: + setState(Worker.State.CANCELLED); + break; + } + }); + return rv.get(); } } - // return the flag return flag; } diff --git a/modules/javafx.graphics/src/test/java/test/javafx/concurrent/ServiceLifecycleTest.java b/modules/javafx.graphics/src/test/java/test/javafx/concurrent/ServiceLifecycleTest.java index ce73a18340f..313ab9cfcaf 100644 --- a/modules/javafx.graphics/src/test/java/test/javafx/concurrent/ServiceLifecycleTest.java +++ b/modules/javafx.graphics/src/test/java/test/javafx/concurrent/ServiceLifecycleTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,9 +25,13 @@ package test.javafx.concurrent; -import test.javafx.concurrent.mocks.MythicalEvent; -import test.javafx.concurrent.mocks.SimpleTask; -import javafx.event.EventHandler; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; @@ -40,17 +44,13 @@ import javafx.concurrent.TaskShim; import javafx.concurrent.Worker; import javafx.concurrent.WorkerStateEvent; - +import javafx.event.EventHandler; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import test.javafx.concurrent.mocks.MythicalEvent; +import test.javafx.concurrent.mocks.SimpleTask; /** * Tests various rules regarding the lifecycle of a Service. @@ -1278,6 +1278,7 @@ public void removed_onSucceededFilterNotCalled() { task.complete(); } + @RepeatedTest(50) @Test public void cancelCalledFromOnSucceeded() { final AtomicInteger cancelNotificationCount = new AtomicInteger(); @@ -1422,6 +1423,7 @@ public void cancelCalledFromOnCancelled() { assertEquals(1, cancelNotificationCount.get()); } + @RepeatedTest(50) @Test public void cancelCalledFromOnFailed() { final AtomicInteger cancelNotificationCount = new AtomicInteger(); From 33143f166d456f9a8ba2e41390c1fa20b11a8049 Mon Sep 17 00:00:00 2001 From: Andy Goryachev Date: Wed, 16 Apr 2025 13:23:13 -0700 Subject: [PATCH 2/3] review comments --- .../src/main/java/javafx/concurrent/Task.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java b/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java index ffde8003ec5..2285077522b 100644 --- a/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java +++ b/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java @@ -31,6 +31,7 @@ import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SCHEDULED; import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SUCCEEDED; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.FutureTask; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -1025,20 +1026,19 @@ public boolean cancel(boolean mayInterruptIfRunning) { setState(Worker.State.CANCELLED); } else { - AtomicBoolean rv = new AtomicBoolean(flag); runLater(() -> { + // the state must be accessed only in the fx application thread switch (getState()) { case FAILED: case SUCCEEDED: // a finished or failed task retains its state - rv.set(false); break; default: setState(Worker.State.CANCELLED); break; } }); - return rv.get(); + return flag; } } return flag; From 66d2edb9e3bc52d642e6f6622d7a4b50e27c8e7c Mon Sep 17 00:00:00 2001 From: Andy Goryachev Date: Wed, 16 Apr 2025 13:23:39 -0700 Subject: [PATCH 3/3] cleanup --- .../javafx.graphics/src/main/java/javafx/concurrent/Task.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java b/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java index 2285077522b..29abb042713 100644 --- a/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java +++ b/modules/javafx.graphics/src/main/java/javafx/concurrent/Task.java @@ -31,9 +31,7 @@ import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SCHEDULED; import static javafx.concurrent.WorkerStateEvent.WORKER_STATE_SUCCEEDED; import java.util.concurrent.Callable; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.FutureTask; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import javafx.application.Platform; import javafx.beans.property.BooleanProperty;