How to implement start and pause functions in JavaFX the correct way?

Question

I'm creating a JavaFX that scans your inbox.

There are 2 buttons, one to start scanning, and one to pause scanning.

To implement this I've created a new thread passing it in a runnable where the scanInbox() function is called.

However, when I press the pause button to invoke thread.wait(), it seems to get stuck.

What would be the best way to implement this function here?

public class WebsiteOverviewController {

    @FXML
    private TableView<Website> deleteTable;

    @FXML
    private TableColumn<Website, String> deleteColumn;

    @FXML
    private TableView<Website> keepTable;

    @FXML 
    private TableColumn<Website, String> keepColumn;

    @FXML
    private JFXButton scanButton;

    @FXML
    private JFXButton pauseButton;

    private BooleanProperty isScanning = new SimpleBooleanProperty(false);

    private MainApp mainApp;

    private FilteredList<Website> keepData;

    private FilteredList<Website> deleteData;

    Task<Void> task;
    Thread thread;


    public WebsiteOverviewController() {

    }

    @FXML
    public void initialize() {
        deleteColumn.setCellValueFactory(cellData -> cellData.getValue().websiteProperty());
        keepColumn.setCellValueFactory(cellData -> cellData.getValue().websiteProperty());

        scanButton.visibleProperty().bind(isScanning.not());
        pauseButton.visibleProperty().bind(isScanning);
    }

    public void setMainApp(MainApp mainApp) {
        this.mainApp = mainApp;

        keepData = new FilteredList<>(mainApp.getWebsiteData(), p -> p.getKeep());
        deleteData = new FilteredList<>(mainApp.getWebsiteData(), p -> !p.getKeep());
        deleteTable.setItems(deleteData);
        keepTable.setItems(keepData);
    }

    @FXML
    public void handleScanInbox() {
        isScanning.set(true);


        thread = new Thread(new Runnable() {

            @Override
            public void run() {
                mainApp.handleScanInbox();
            }

        });
        thread.start();


    }

    @FXML
    public void handlePauseScanInbox() {
        isScanning.set(false);
        try {
            synchronized(thread) {
                thread.wait();
            }

        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }


}

Answer 1

Alex's solution is working fine, but if you still want to use a lower-level of threading management (wait-notify), this is how you can do it:

public class WebsiteOverviewController {
    @FXML
    public void handleScanInbox() {
        isScanning.set(true);

        thread = new Thread(mainApp::handleScanInbox);
        thread.start();
    }

    @FXML
    public void handlePauseScanInbox() {
        isScanning.set(false);
        mainApp.pause();
    }

    // Another handler for resuming...
}

public class MainApp {
    private final AtomicBoolean paused = new AtomicBoolean(false);

    public void handleScanInbox() {
        for (int i = 0; i < numberOfItems; i++) { // This could be a while loop
            synchronized (paused) {
                while (paused.get()) { // Using while so that it could re-wait in the case that the object was falsely notified
                    try {
                        pause.wait();
                    }
                    catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }

            // Do whatever you need to do
        }
    }

    public void pause() {
        pause.compareAndSet(false, true);
    }

    public void resume() {
        synchronized (paused) {
            if (paused.get()) {
                paused.set(false);
                paused.notify();
            }
        }
    }
}

Update

If you want to be able to call a particular to toggle between pausing and resuming, you can add another method for this:

// In MainApp
public void togglePauseResume() {
    synchronized (paused) {
        if (paused.get()) {
            paused.set(false);
            paused.notify();
        }
        else {
            paused.set(true); // You don't need compareAndSet() because paused object has been used for synchronization (i.e. locked)
        }
    }
}

In any case, you should try to avoid this:

@FXML
public void handleButton() {
    if (mainApp.isPaused()) { // You added a getter for paused
        mainApp.pause();
    }
    else {
        mainApp.resume();
    }
}

This is because MainApp.paused can potentially change in between the getter and pause() (i.e. race condition).

Update 2

If you just want to use a single method to start/resume the thread, you could simply create the thread if it is null, or call resume() otherwise.

@FXML
public void handleScanInbox() {
    isScanning.set(true); // Not sure if you still need this

    if (thread == null) {
        thread = new Thread(mainApp::handleScanInbox);
        thread.start();
    }
    else if (thread.isAlive()) {
        mainApp.resume();
    }
}

I have also changed if (paused.get()) to while (paused.get()), in case notify() was called on paused by accident.