-2

I have a single scene JavaFX app, that runs an animation, the animation needs to switch back and forth between two states.
Based on an example in "Java How to Program, 11/e, Early Objects" I have written a controller that creates a similar setup in the initialization method,
And a task with boolean value that signals the animation timer when to switch states by fliping its value and then sleep.
I keep getting "java.lang.IllegalStateException: Task must only be used from the FX Application Thread" thrown from the call() method no metter what I do.

here is a simplified version of the controller:

public class AnimationController {
    @FXML public AnimationTimer myAnimationTimerExtention;
    private ExecutorService executorService;

   public void initialize() {
        myAnimationTimerExtention.setState(false);
        TimeingTask task = new TimeingTask (Duration.ofSeconds(5));

        task .valueProperty().addListener((observableValue, oldValue, newValue) -> {
            myAnimationTimerExtention.setState(false);
        });

        executorService = Executors.newFixedThreadPool(1);
        executorService.execute(timeTrafficLightTask);
        executorService.shutdown();
    }

And here is my task:

public class TimeingTask  extends Task<Boolean> {
    private final Duration duration;

    public TimeingTask(Duration duration) {
        this.duration = duration;
        updateValue(true);
    }

    @Override
    protected Boolean call() throws Exception {
        while (!isCancelled()) {
            updateValue(!getValue());
            try {
                Thread.sleep(duration);
            } catch (InterruptedException e) {
                updateMessage("timing task got interrupted");
            }

        }

        return true;
    }
}

I have tried so far:

  1. moveing the initialization to be done in a button's action
  2. removing the listener and the update of the value
  3. verifing with Thread.currentThread().getName() I'm on the application theard in the controller.
  4. chack if the task somehow get canceld to see if the issue relates to this javafx bug, it is not getting canceld.
  5. compiling and running the example I'm basing this on to see if this is realted to my IDE configuration or JDK version, the example worked.
  6. surrounding the updateValue() in a Platform.runLater() the excption is thrown on the Theard.sleep()

in all of the above scenarios the excption was thrown

  • 2
    Create and post a [mre], including the complete stack trace (formatted as code). – James_D May 12 '23 at 16:14
  • Probably off-topic, but 1. Are you really creating an `AnimationTimer` subclass instance in your FXML? That's possible but seems to be a really strange thing to do. 2. Why create an `ExecutorService` just to use it once and immediately shut it down? You may as well just create a `Thread` directly. – James_D May 12 '23 at 16:17
  • 2
    Also, I suppose, why use threads at all here? Why not just use the animation API? See https://stackoverflow.com/a/60685975/2189127 – James_D May 12 '23 at 16:19
  • 1
    Re, "...despite task created...from the FX Application Thread." It doesn't matter which thread created the `Task` object. What matters is which thread calls `getValue()` (see James_D's answer, below). – Solomon Slow May 12 '23 at 17:53
  • 2
    Re, "...despite task...ran from the FX Application Thread." But, you _didn't_ run it from the application thread. You ran it from a worker thread belonging to your `executorService`. – Solomon Slow May 12 '23 at 17:54
  • 3
    Re, `es=Executors.newFixedThreadPool(1); es.execute(task); ex.shutdown();` That's pure [_cargo cult programming_](https://en.wikipedia.org/wiki/Cargo_cult_programming). If you think you need to create a one-shot thread, then call `new Thread(...)`. If somebody told you that calling `new Thread(...)` was a bad idea, it's because they thought that _needing a one-shot thread_ was a bad idea. What you did—creating a new thread _pool_ just so that you could submit _one task_ to it, is no better than `new Thread(...)`. In fact, it's just a tiny bit _worse._ – Solomon Slow May 12 '23 at 18:00
  • Does this answer your question? [Change javafx circles color in certain time using multithreads](https://stackoverflow.com/questions/52589640/change-javafx-circles-color-in-certain-time-using-multithreads) – SedJ601 May 12 '23 at 18:48
  • @James_D 1. my real code has an exstantion of canvas that draws some stuff I need and has the animation timer moving those stuff, and I have more then one of those, whice is way it's an extention of canvas. I was trying to simplifiy it to the bare necessities and only the animation timer seemd relevent. 2. you are right, I was using the example in the book as a guidline and diden't think it through – WildLAppers May 12 '23 at 19:50
  • @SolomonSlow It was clear to me that the task itself is ran in a background theard, what I was trying to say is that I have started it from the application thread, which I tought is a requirement that the excption message was trying to convey. I had no idea getValue() can only be called from the application thread and googling the error did not yield this information. as for the thread pool issue, you are right, it's an over kill. – WildLAppers May 12 '23 at 20:01
  • 1
    @SedJ601 thanks, but qustion in the link did not help me, the writer of the other question is trying to achive similar result, but he used another way then I did, and I was intrested in understanding why my way did not work. – WildLAppers May 12 '23 at 20:04
  • @WildLAppers, I don't have JavaFX experience, but I would be mildly surprised if it mattered which thread constructed the object, and I would be _extremely_ surprised if it mattered which other thread `start()`ed the thread that ultimately uses the object. – Solomon Slow May 12 '23 at 20:07
  • @SolomonSlow I'm also just starting with JavaFX, from what I understand from the textbook I am using, the Task class was written especially for JavaFX. So despite the fact it is a weird requiremet (if it was a requirement), it didn't seemed out of the realm of possiblitiy – WildLAppers May 12 '23 at 20:17
  • 2
    Interrupts don’t happen by accident. If someone interrupts a thread, it’s because they are asking that thread to stop what it’s doing and terminate cleanly. Ignoring an interrupt is the worst possible response—it means your thread is a rogue thread that can never be halted. The correct course of action is to exit your while loop when you receive an interrupt. (An easy to way to do that is to remove the try/catch and put the entire while loop inside a try/catch.) – VGR May 12 '23 at 20:35
  • 1
    how about studying the java doc of classes (vs random experiments) you are using? - it's very clear in describing your _exact_ mis-use of getXX and its _exact_ consequence you are seeing – kleopatra May 13 '23 at 00:43
  • @kleopatra actually, I have read the [documantation](https://docs.oracle.com/javase/8/javafx/api/javafx/concurrent/Task.html#getValue--), and it makes no mantion of this beaviour, I've looked for more, and for that excption specifically, and found nothing. So I am unsure what you are referring to – WildLAppers May 13 '23 at 13:05
  • read the class doc .. – kleopatra May 13 '23 at 14:39
  • @kleopatra I linked the section of the documention describing the method, but I have read the top documention describeing the class as well, I've just read it again and it has no mention of this behavior. – WildLAppers May 13 '23 at 15:02
  • 3
    @WildLAppers You didn't read it very carefully. It explicitly says: *"Because the Task is designed for use with JavaFX GUI applications, it ensures that every change to its public properties, as well as change notifications for state, errors, and for event handlers, all occur on the main JavaFX application thread. **Accessing these properties from a background thread (including the call() method) will result in runtime exceptions being raised.**"*. (My emphasis). – James_D May 13 '23 at 16:34
  • And I think you missed my point about the `AnimationTimer`. The point isn't whether or not your application needs it, but why it's defined in the FXML file. Typically you use the FXML file to define actual UI components (subclasses of `Node`); and animation timer is essentially executable code, not something you lay out. – James_D May 13 '23 at 17:40
  • @James_D I got your point about the `AnimationTimer` , what I was saying is that in my actual code the class in the FXML is an extension of `Canvas` , which is a subclass of `Node`. The `Canvas` extension has an internal `AnimationTimer`, the timer seemed relvenet to the issue since it mutates UI values, while the canvas was not. So for the question sake, I simplified the canvas out of the code and left the timer. In hindsight, I should have focused on the `Task` in my question. – WildLAppers May 14 '23 at 07:12

1 Answers1

6

The exception is being thrown because you can only directly access the value property from the FX Application Thread. You can call updateValue() from any thread, but you cannot call getValue() from a background thread. If you inspect your stack trace carefully, you should see something like:

java.lang.IllegalStateException: Task must only be used from the FX Application Thread
  at javafx.concurrent.Task.checkThread()
  at javafx.concurrent.Task.getValue()
  at yourpackage.TimeingTask.call()

meaning that the exception is thrown by the call to getValue() (not the call to updateValue()) in your call() method.

The implementation of Task.getValue() is

@Override public final V getValue() { checkThread(); return value.get(); }

with

private void checkThread() {
    if (started && !isFxApplicationThread()) {
        throw new IllegalStateException("Task must only be used from the FX Application Thread");
    }
}

And, indeed, the documentation explicitly states

Because the Task is designed for use with JavaFX GUI applications, it ensures that every change to its public properties, as well as change notifications for state, errors, and for event handlers, all occur on the main JavaFX application thread. Accessing these properties from a background thread (including the call() method) will result in runtime exceptions being raised.

(My emphasis.)

So when you invoke getValue() from your call() method (which of course is run on the background thread), it throws the IllegalStateException.


Threads are greatly overkill for what you're trying to do here anyway. As described here, you can greatly simplify this with a Timeline:

public void initialize() {
    myAnimationTimerExtention.setState(false);
    KeyFrame frame = new KeyFrame(Duration.ofSeconds(5), e -> 
        myAnimationTimerExtention.setState(! myAnimationTimerExtention.getState()));
    Timeline timeline = new Timeline(frame);
    timeline.setCycleCount(Animation.INDEFINITE);
    timeline.play();

}

Or just build the functionality directly into your AnimationTimer, which could easily check the timestamp passed to the handle() method and flip the state if needed.

This gets rid of the need for your Task subclass, the Executor, and means the application doesn't consume additional resources by creating an additional thread.


Just in case the code in the question is a simplification, and you really need to use a background thread for some unknown reason, just shadow the value. To emphasize: do not add all the complexity of multithreading unless you need it. The solution above is a far better solution for the functionality you described.

public class TimeTrafficLightTask  extends Task<Boolean> {
    private final Duration duration;
    private boolean state = true ;

    public TimeTrafficLightTask(Duration duration) {
        this.duration = duration;
        updateValue(state);
    }

    @Override
    protected Boolean call() throws Exception {
        while (!isCancelled()) {
            state = !state ;
            updateValue(state);
            try {
                Thread.sleep(duration);
            } catch (InterruptedException e) {
                updateMessage("timing task got interrupted");
            }

        }

        return true;
    }
}
James_D
  • 201,275
  • 16
  • 291
  • 322
  • you are right about threading being an over kill, however, this is for an assigment and the assignment specifically requests a thread to synchronize the states – WildLAppers May 12 '23 at 19:40
  • 3
    @WildLAppers when you have weird requirements like that, you should state them in the question. It would have saved a lot of time and confusion. – jewelsea May 12 '23 at 21:18