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() {
//...
}
}
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 besynchronized
because they are not doing a safe publishing of thereplenishTask
field. That field is notfinal
and the compiler is allowed to havecreate(...)
return but continue the construction and initialization of theEnergySource
object a later.If multiple threads were creating and stopping these objects, you could easily get a NPE in
stopEnergySource(...)
, even with thesynchronized
keyword on it.