1

When I had my logic inside a Runnable it worked fine except I could not interact with the UI Thread. So I am trying to put everything inside a class that extends Task and it works Except the task is only executed once. No errors and I get a succeeded message form the Task succeeded method.

I have also tried making the task return Boolean true in the call method but that did not help.

public class Main extends Application { 
    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) throws Exception{   
        SyncTask syncTask = new SyncTask();

        ScheduledExecutorService executor = Executors.newScheduledThreadPool(1);
        executor.scheduleAtFixedRate(syncTask, 0, 10, TimeUnit.SECONDS);

        Label syncEngineLabel = centralController.getScheduleTabMessageLabel();
        syncEngineLabel.textProperty().bind(syncTask.messageProperty());
    }
    class SyncTask extends Task<Void>{
        private Schedule schedule = null;

        public SyncTask() {}

        @Override
        protected Void call() throws Exception {
            System.out.println("we are in the task...");
            if (getScheduleFromApi()){
                updateMessage("New Schedule Retrieved...");
            }
            return null;
        }
        @Override protected void succeeded() {
            super.succeeded();
            System.out.println("succeeded");
        }

        @Override protected void cancelled() {
            super.cancelled();
            System.out.println("cancelled");
        }

        @Override protected void failed() {
            super.failed();
            System.out.println("failed");
        }

        private Boolean getScheduleFromApi(){
            Gson gson = new GsonBuilder().serializeNulls().create();
            ApiGet api = new ApiGet("schedule/get-schedule-by-room", parameters);
            api.sendRequest();

            if (api.isSuccess()){
                schedule = gson.fromJson(api.response(), Schedule.class);
                if (schedule.getStatus().equals("200")){
                    return true;
                }
                else{
                    updateMessage(schedule.getMsg());
                    return false;
                }
            }
            else {
                updateMessage("Failed to process API call to ASI Server.");
                return false;
            }
        }
    }
}

Please note that this code actually exists inside a controller but I put it in Main here to try and provide self contained code.

Thanks!

Daniel H.
  • 422
  • 2
  • 15
  • You're trying to execute the same task instance multiple times. – James_D Oct 25 '17 at 17:48
  • When I try to (?instantiate?) inside the executer it says expression expected. How do i create a new instance for each execution? and why cant I run the same instance over and over? `executor.scheduleAtFixedRate(SyncTask, 0, 10, TimeUnit.SECONDS);` – Daniel H. Oct 25 '17 at 17:52
  • You can't reuse a `Task`, according to the [documentation](https://docs.oracle.com/javase/9/docs/api/javafx/concurrent/Task.html). (Basically it would violate the state transition specifications of the `Task` class to go, say, from `SUCCEEDED` to `READY`, etc.) Use a `ScheduledService`. – James_D Oct 25 '17 at 17:53
  • ok.. Is there a way for me to create a new instance within the executor or do i need to create a method that gets called and creates the instance? – Daniel H. Oct 25 '17 at 17:55

1 Answers1

3

The ScheduledExecutorService will simply treat the task you provide as a Runnable, and try to reuse the same task instance every time it runs, which is explicitly forbidden in the documentation.

Use a ScheduledService instead:

@Override
public void start(Stage primaryStage) throws Exception{   

    ScheduledService<Void> scheduledService = new ScheduledService<Void>() {
        @Override
        protected Task<Void> createTask() {
            return new SyncTask();
       }
    };
    scheduledService.setPeriod(Duration.seconds(10));
    scheduledService.start();

    Label syncEngineLabel = centralController.getScheduleTabMessageLabel();
    scheduledService.stateProperty().addListener((obs, oldState, newState) -> {
            if (newState == Worker.State.RUNNING) {
                syncEngineLabel.setText("Sync in progress");
            } else if (newState == Worker.State.FAILED) {
                syncEngineLabel.setText("Sync error");
            } else {
                syncEngineLabel.setText("Sync complete");
            }
    });


}
James_D
  • 201,275
  • 16
  • 291
  • 322
  • Thanks James_D. I had to change Task to SyncTask. My only other question, (I am new to threading) if one of the tasks that gets started is long running will a newly created task go into a waiting que for the first one to finish or will it be started on its own thread? – Daniel H. Oct 25 '17 at 18:11
  • 1
    @DanielH. The former (it waits). According to the documentation: "The amount of time the `ScheduledService` will remain in [the `SCHEDULED`] state depends on the amount of time between the last state transition to `RUNNING`, and the current time, and the period. ... If the previous execution completed before period expires, then the `ScheduledService` will remain in the `SCHEDULED` state until the period expires. If on the other hand the execution took longer than the specified period, then the `ScheduledService` will immediately transition back to `RUNNING`." – James_D Oct 25 '17 at 18:16
  • got it. I hope this isn't annoying, but one more question. Any idea why messageProperty is not being updated? I am getting nothing in my label. – Daniel H. Oct 25 '17 at 18:27
  • @DanielH. I believe the message property of a `Service` resets to `null` when the service is not running a task. Since you only update the task's message immediately before the task exits, you don't see anything. You might want to observe the state of the service instead: see update to answer. – James_D Oct 25 '17 at 18:35
  • Ahh..Knowing it resets to null clears up my confusion. The listener helps. I will play around with it. Thanks again!!!!!!! – Daniel H. Oct 25 '17 at 18:44