ScheduledExecutorService running a couple of scheduleAtFixedRate

Question

I have created a task that I need to query database for List of String and store it in neededDataList. I will use neededDataList in a scheduled task every 10 minutes to do a simple task.

And I need to retrieve a fresh data of neededDataList every 1 hour. Will the scheduledExecutorService2 pick up the fresh neededDataList? Can someone review my code if it will cause some problem?

Assuming my code has its main method and it will call the run(Database database):

    public class Handler {
            
            private static final ScheduledExecutorService scheduledExecutorService1 = MDScheduledExecutorService.wrap(
                        new ScheduledThreadPoolExecutor(1), "ses-1-thread-%d");
            private static final ScheduledExecutorService scheduledExecutorService2 = MDScheduledExecutorService.wrap(
                        new ScheduledThreadPoolExecutor(5), "ses-2-thread-%d");
        
            private List<String> neededDataList;
        
            private void run(Database database) {
                scheduledExecutorService1.scheduleAtFixedRate(() -> {
                    neededDataList= database.getNeededData();
                }, 1, 60, TimeUnit.MINUTES);
                
                scheduledExecutorService2.scheduleAtFixedRate(new HandlerImpl(), 1, 10, TimeUnit.MINUTES);
            }
    
            private class HandlerImpl implements Runnable {
            
            @Override
            public void run() {
               executeTask(neededDataList);
            }
        
        
            }
}

Answer 1

Your code is not thread-safe.

The Handler.run() method creates two services which will end up being separate threads. These threads are accessing and updating the neededDataList shared variable without any synchronization. This could lead to unpredictable behavior, including the service that is reading the list seeing stale values.

Also, this code won't compile because the executeTask method is not defined.

Answer 2

Extending the answer of @StephenC:

You could wrap neededDataList in a java.util.concurrent.atomic.AtomicReference. This only works out well if your executeTask method always only reads from the neededDataList and doesn't modify the list. If modifications are required, you could have a look at java.util.concurrent.CopyOnWriteArrayList.

No performance considerations done here, just what is necessary to prevent crashes or deadlocks.