2

So I got a rather simple JavaFX application that creates and starts 2 threads when pressing a start button. I also have a reset button which should interrupt the threads and clear the GUI components. This seems to work fine. Then I should be able to press the reset button and then start button again to create 2 threads and start them again. This also works. When these threads are executing they are using my callback, to give back string to my Controller class, which then adds it to a listview. This works fine the first time I run the application, but if I press clear and then start, the callback throws error when adding data to the listview. java.lang.IllegalStateException: Not on FX application thread

As I understand this is thrown whenever some other thread than main tries to change a fx element. But why does it work fine the first time I click start, but not after clicking clear and then start again? This is my code:

/**
 * Controls GUI and start threads..
 * 
 *
 */
public class Controller implements Initializable {
    @FXML
    private Button btnRun;

    @FXML
    private Button btnReset;

    @FXML
    private TextArea taInput;

    @FXML
    private ListView<String> lwProducer;

    @FXML
    private ListView<String> lwConsumer;

    private boolean sync;
    private Buffer buffer;
    private ObservableList<String> conData;
    private ObservableList<String> proData;
    private Thread consumerThread;
    private Thread producerThread;
    private Runnable producerR;
    private Runnable consumerR;

    /**
     * Initializes elements, sets an initial value for the textbox.
     */
    public void initialize(URL location, ResourceBundle resources) {
        taInput.setText("The quick brown\nfox jumps over\nthe lazy dog");
        btnReset.setDisable(true);
        btnRun.setDefaultButton(true);
        lwProducer.setStyle("-fx-focus-color: transparent;");
        lwConsumer.setStyle("-fx-focus-color: transparent;");
    }

    /**
     * Eventhandler for btnRun that will start new threads.
     * 
     * @param e
     */
    @FXML
    protected void btnRun(ActionEvent e) {
        System.out.println(sync);
        conData = FXCollections.observableArrayList();
        proData = FXCollections.observableArrayList();

        buffer = new Buffer();

        producerR = new Producer(new ProducerCB(), buffer, taInput.getText());
        consumerR = new Consumer(new ConsumerCB(), buffer);

        producerThread = new Thread(producerR);
        consumerThread = new Thread(consumerR);

        producerThread.start();
        consumerThread.start();

        btnRun.setDisable(true);
        taInput.setDisable(true);
        btnReset.setDisable(false);
    }

    /**
     * Eventhandler for btnReset. If pressed, threads and all gui components
     * will be cleared.
     * 
     * @param e
     */
    @FXML
    protected void btnReset(ActionEvent e) {
        System.out.println("Resetting");
        producerThread.interrupt();
        consumerThread.interrupt();
        producerThread = null;
        consumerThread = null;
        lwProducer.getItems().clear();
        lwConsumer.getItems().clear();
        conData = null
        proData = null
        producerR = null;
        consumerR = null;
        btnRun.setDisable(false);
        taInput.setDisable(false);
        btnReset.setDisable(true);
    }

    /**
     * Callback for producer thread. Updates the left hand listview with added
     * data.
     * 
     *
     */
    private class ProducerCB implements Callback {
        @Override
        public void returnData(String str) {
            proData.add(str);
            lwProducer.setItems(proData);
        }
    }

    /**
     * Callback for consumer thread. Updates the right hand listview with read
     * data.
     * 
     */
    private class ConsumerCB implements Callback {
        @Override
        public void returnData(String str) {
            conData.add(str); // Throws exception!!!!!!!
            lwConsumer.setItems(conData);
        }
    }
}

Stacktrace

Exception in thread "Thread-7" java.lang.IllegalStateException: Not on FX application thread; currentThread = Thread-7
    at com.sun.javafx.tk.Toolkit.checkFxUserThread(Toolkit.java:236)
    at com.sun.javafx.tk.quantum.QuantumToolkit.checkFxUserThread(QuantumToolkit.java:423)
    at javafx.scene.Parent$2.onProposedChange(Parent.java:367)
    at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:113)
    at com.sun.javafx.collections.VetoableListDecorator.setAll(VetoableListDecorator.java:108)
    at com.sun.javafx.scene.control.skin.LabeledSkinBase.updateChildren(LabeledSkinBase.java:575)
    at com.sun.javafx.scene.control.skin.LabeledSkinBase.handleControlPropertyChanged(LabeledSkinBase.java:204)
    at com.sun.javafx.scene.control.skin.ListCellSkin.handleControlPropertyChanged(ListCellSkin.java:49)
    at com.sun.javafx.scene.control.skin.BehaviorSkinBase.lambda$registerChangeListener$61(BehaviorSkinBase.java:197)
    at com.sun.javafx.scene.control.MultiplePropertyChangeListenerHandler$1.changed(MultiplePropertyChangeListenerHandler.java:55)
    at javafx.beans.value.WeakChangeListener.changed(WeakChangeListener.java:89)
    at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:182)
    at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
    at javafx.beans.property.StringPropertyBase.fireValueChangedEvent(StringPropertyBase.java:103)
    at javafx.beans.property.StringPropertyBase.markInvalid(StringPropertyBase.java:110)
    at javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:144)
    at javafx.beans.property.StringPropertyBase.set(StringPropertyBase.java:49)
    at javafx.beans.property.StringProperty.setValue(StringProperty.java:65)
    at javafx.scene.control.Labeled.setText(Labeled.java:145)
    at com.sun.javafx.scene.control.skin.ListViewSkin$2.updateItem(ListViewSkin.java:319)
    at javafx.scene.control.ListCell.updateItem(ListCell.java:471)
    at javafx.scene.control.ListCell.lambda$new$160(ListCell.java:167)
    at javafx.collections.WeakListChangeListener.onChanged(WeakListChangeListener.java:88)
    at com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:329)
    at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
    at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
    at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
    at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
    at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
    at javafx.collections.ModifiableObservableListBase.add(ModifiableObservableListBase.java:155)
    at java.util.AbstractList.add(AbstractList.java:108)
    at assignment2.Controller.addData(Controller.java:163)
    at assignment2.Controller.access$2(Controller.java:162)
    at assignment2.Controller$ConsumerCB.returnData(Controller.java:158)
    at assignment2.Consumer.run(Consumer.java:22)
    at java.lang.Thread.run(Thread.java:745)
korv
  • 19
  • 1
  • 3
  • Could you please post the stack trace? – keuleJ Nov 16 '16 at 19:23
  • Well @GhostCat is right, your accessing FX-Contronls on the wrong thread ("Thread-7" instead of the JavaFX-Thread). – keuleJ Nov 16 '16 at 19:33
  • If you read the question, you would see that I already know that. – korv Nov 16 '16 at 19:34
  • This is probably by accident, as you really just add something to the List. But this is an observable list and you have bound it to some FX-Control via the @FXML Annotation – keuleJ Nov 16 '16 at 19:35
  • I would recommend to read about concurrency and JavaFX: https://docs.oracle.com/javase/8/javafx/interoperability-tutorial/concurrency.htm It is explained how to pass results from other Threads... – keuleJ Nov 16 '16 at 19:39
  • "As I understand this is thrown whenever some other thread than main tries to change a fx element." Not quite: the word "whenever" is inaccurate here. The exception *may* be thrown, but is not guaranteed to be thrown, if you violate JavaFX threading rules. The framework makes a best attempt to fail as soon as possible, but performance considerations (among others) prevent checks from being made for every possible violation. – James_D Nov 16 '16 at 19:53
  • There's just too much wrong here to really figure out exactly why you're seeing the behavior you see, but here's a guess. Your `conData` reference is shared between multiple threads without any attempt at synchronization whatsoever. You assign it a value on the FX Application Thread in `btnRun(...)` and then reference it in another thread in `ConsumerCB.returnData(...)`. My guess is that on the second invocation, the background thread is seeing a stale value of that reference. (The new list you expect it to refer to is not connected to a UI control, so this seems to make sense.) – James_D Nov 16 '16 at 20:07
  • A couple of resources: SO user @jewelsea has a couple of examples that I consider to be among the best JavaFX/multithreading examples available. These are pre-Java8, so they could be "modernized" a bit, but you will get the concepts. https://gist.github.com/jewelsea/5500981 and https://gist.github.com/jewelsea/5072743 – James_D Nov 17 '16 at 00:38

2 Answers2

6

Similar to the AWT event dispatcher thread, you can only update JavaFx UI components when using that very Java FX application thread.

See here for further reading on this topic. One way of resolving such problems is by using Platform.runLater().

jewelsea
  • 150,031
  • 14
  • 366
  • 406
GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • I already know this. I tried Platform.runLater but this gives med weird results. My synchronization stops working. – korv Nov 16 '16 at 19:35
  • 3
    Then your sync code is probably buggy. The point is: when your code doesn't work as expected, that doesn't mean that the concept you are using is wrong. It means that there are bugs to find and fix! – GhostCat Nov 16 '16 at 19:36
  • Not impossible. :) But I look at my Task Manager, each time I press reset and start, my CPU usage goes up and stays up, so my old threads doesn't seem to get killed. Edit: Just did a small conditional in me reset to check if the threads are alive, and indeed my consumer thread is not interrupted. :S – korv Nov 16 '16 at 19:39
  • 2
    Sure: your producer/consumers are **not** sleeping. They are **hot** all the time. That means that they constantly try to do something. In other words: they put 100% load on your CPU. Long story short: you problem is probably more on the "you don't know what you are doing level". I would rather recommend to sit down with some peers and review/talk about your code in detail. But maybe you are lucky and somebody comes by who has the time and motivation to dig into your code. But not me today. – GhostCat Nov 16 '16 at 19:48
0

According to the Stacktrace, you are changing some fx element's value in non fx thread(called "Thread-7“, obviously this is a anonymous thread you created). I think you can set the break point in conData.add(str) and see the call hierarchy. You will find the root cause by your own.

truejasonxiefans
  • 179
  • 2
  • 12