Synchronize of ScheduledFuture.cancel() method

409 views Asked by At

Code below is cutted example from "Programming Concurrency on the JVM: Mastering Synchronization, STM, and Actors" book

I don't understand why author synchronizing stopEnergySource method, which just cancels ScheduledFuture task, which represented by replenishTask variable? There are no other methods that use this variable. Is it desirable practice to synchronize Future.cancel calls or it just needed for ScheduledFuture?

public class EnergySource {
  //...
  private static final ScheduledExecutorService replenishTimer =
    Executors.newScheduledThreadPool(10);
  private ScheduledFuture<?> replenishTask;

  private EnergySource() {}

  private void init() {   
    replenishTask = replenishTimer.scheduleAtFixedRate(new Runnable() {
      public void run() { replenish(); }
    }, 0, 1, TimeUnit.SECONDS);
  }

  public static EnergySource create() {
    final EnergySource energySource = new EnergySource();
    energySource.init();
    return energySource;
  }

  public long getUnitsAvailable() { 
    //...
  }

  public long getUsageCount() { 
    //...
  }

  public boolean useEnergy(final long units) {
    //...
  }

  public synchronized void stopEnergySource() { // what for **synchronized** is?
    replenishTask.cancel(false); 
  }

  private void replenish() { 
    //...
  }
} 
1

There are 1 answers

0
Gray On

There are no other methods that use this variable. Is it desirable practice to synchronize Future.cancel calls or it just needed for ScheduledFuture?

I believe that the author of the code is trying to force memory barriers in case a different thread is trying to call stopEnergySource() that started the job in the first place.

However, if this was the case then the init() method should also be synchronized because they are not doing a safe publishing of the replenishTask field. That field is not final and the compiler is allowed to have create(...) return but continue the construction and initialization of the EnergySource object a later.

If multiple threads were creating and stopping these objects, you could easily get a NPE in stopEnergySource(...), even with the synchronized keyword on it.