JavaFX & Multithreading: IllegalStateException on ObservableList.add()

Question

I have a JavaFX application that communicates with and gets data from a server. The received data is put into an ObservableList and displayed in a TableView.

Communication with the server runs in its own Thread and when calling ObservableList.add it triggers an IllegalStateException (complaining about not being the Event Thread / not being the JavaFX Thread)

I have found the following solution to a similar problem, but I am not sure how I could adopt this as in my case the communication with the server needs to be held up constantly, so the task/thread is running until the communication is terminated.

I have a minimum working example here that triggers said exception and roughly models how the application works.

Main:

package sample;

import javafx.application.Application;
import javafx.fxml.FXMLLoader;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.stage.Stage;

public class Main extends Application {

    @Override
    public void start(Stage primaryStage) throws Exception{
        Parent root = FXMLLoader.load(getClass().getResource("sample.fxml"));
        primaryStage.setTitle("Hello World");
        primaryStage.setScene(new Scene(root, 300, 275));
        primaryStage.show();
    }


    public static void main(String[] args) {
        launch(args);
    }
}

Controller:

package sample;

import javafx.beans.property.SimpleStringProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.concurrent.Task;
import javafx.fxml.FXML;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;

public class Controller {
    public TableView<Integer> timeTable;
    public TableColumn<Integer, String> positionColumn;

    private ObservableList<Integer> testList;

    @FXML
    private void initialize() {
        testList = FXCollections.synchronizedObservableList(FXCollections.observableArrayList());
        positionColumn.setCellValueFactory(cellData -> new SimpleStringProperty(cellData.getValue().toString()));
        timeTable.setItems(testList);
        Task<Integer> integerTask = new Test(testList);
        Thread testThread = new Thread(integerTask);

        testThread.start();
    }
}

Communication Task:

package sample;

import javafx.collections.ObservableList;
import javafx.concurrent.Task;

public class Test extends Task<Integer> {
    private ObservableList<Integer> testlist;

    Test(ObservableList<Integer> list) {
        testlist = list;
    }

    @Override
    protected Integer call() throws Exception {
        // Emulates the server communication thread. Instead of an endless loop, I used a fixed number of iterations.
        // The real application has an endless while loop for server communication so a Task cannot be used to
        // get the data

        // getDataFromServer()
        // parseData()
        // putDataInList()
        // loop
        Thread.sleep(2000);
        for (int i = 0; i < 500; ++i) {
            testlist.add(i);
        }

        return 0;
    }
}

FXML:

<?import javafx.scene.control.*?>
<?import javafx.scene.layout.GridPane?>
<GridPane fx:controller="sample.Controller"
          xmlns:fx="http://javafx.com/fxml" alignment="center" hgap="10" vgap="10">
    <TableView fx:id="timeTable" editable="true" prefHeight="498.0"
               prefWidth="254.0">
        <columns>
            <TableColumn fx:id="positionColumn" prefWidth="73.0" text="Position"/>
        </columns>
        <columnResizePolicy>
            <TableView fx:constant="CONSTRAINED_RESIZE_POLICY"/>
        </columnResizePolicy>
    </TableView>
</GridPane>

Answer 1

Even though the list is synchronized modifications of the list still trigger events on the thread that does the modifications. In your case this results in the TableView updates being triggered on a non-application thread.

Furthermore you cannot simply use Task.updateValue, see the following section of the javadoc (emphasis mine):

Calls to updateValue are coalesced and run later on the FX application thread [...] and intermediate values may be coalesced to save on event notifications.

You need to synchronize this yourself. The following class combines updated to compensates for fast updates that could potentially slow down the application thread by posting many runnables:

public abstract class JavaFXWorker<T> implements Runnable {

    private final List<T> results = new LinkedList<>();
    private final Object lock = new Object();
    private boolean updateWaiting = false;

    protected void publish(T... values) {
        synchronized (lock) {
            for (T v : values) {
                results.add(v);
            }

            // don't trigger additional updates, if last update didn't fetch the results yet
            // this reduces the number of Runables posted on the application thread
            if (!updateWaiting) {
                updateWaiting = true;
                Platform.runLater(this::update);
            }
        }
    }

    private void update() {
        List<T> chunks;
        synchronized(lock) {
            // copy results to new list and clear results
            chunks = new ArrayList(results);
            results.clear();
            updateWaiting = false;
        }
        // run ui updates
        process(chunks);
    }

    protected abstract void process(List<T> chunks);

}

Your code could be rewritten as follows using the above class.

public class Test extends JavaFXWorker<Integer> {
    private final ObservableList<Integer> testlist;

    public Test(ObservableList<Integer> list) {
        testlist = list;
    }

    @Override
    public void run() {
        // Emulates the server communication thread. Instead of an endless loop, I used a fixed number of iterations.
        // The real application has an endless while loop for server communication so a Task cannot be used to
        // get the data

        // getDataFromServer()
        // parseData()
        // putDataInList()
        // loop
        Thread.sleep(2000);
        for (int i = 0; i < 500; ++i) {
            publish(i);
        }
    }

    @Override
    protected process(List<Integer> chunks) {
        testlist.addAll(chunks);
    }
}

testList = FXCollections.observableArrayList();

...

Thread testThread = new Thread(new Test(testList));

testThread.start();

Answer 2

Instead of updating the ObservableList<Integer> directly in your implementation of the call() method of your Task<Integer>, use updateValue() to publish the new value, as shown here for a Task<Canvas>. A suitable ChangeListener can then update the list safely on the JavaFX Application thread, as discussed here by @James_D.