Working around the "variable might not have been initialized" error

Question

public final void sendAdvertisement(final Advertisement advertisment, int delay, final int repetitions){
    final ScheduledFuture exec = executor.scheduleAtFixedRate( //<< initialized on this line
        new Runnable(){
            int totalSends = 0;
            public void run(){
                //do stuff here

                if(++totalSends >= repetitions) exec.cancel(true); //<< here is says exec might not be initialized
            }
        },
    0, delay, TimeUnit.MILLISECONDS);
}

If this isn't possible, could you suggest a better way to do this? I couldn't find a method for it in ScheduledThreadPoolExecutor. basically what I'm trying to do is make this code be run 3 times then cancel the "timer." I could probably use Swing Timer, but I don't want to since its used for other stuff.

It says in the comments, but this is the error:

%PATH%Discovery.java:148: variable exec might not have been initialized
                if(++totalSends >= repetitions) exec.cancel(true);

Answer 1

Your code may work from the standpoint of removing the compiler warning but the whole point of the warning is pointing out that you may be accessing a variable that has not been assigned yet. Even if exec or exec[0] is non-null there is also no guarantee that the ScheduledFuture object has even been properly initialized -- yes even though the inner thread may be running. This is very dangerous and might work for a while but then fail dramatically in production, when you move to an architecture with more cores, or under different load circumstances. It also may work but then you change your do stuff here code a month from now and it starts to fail.

I see a couple of ways that you can accomplish this in a better manner. They are more complicated but also more safe and consistent with Java. The first that comes to mind is by using the AtomicReference:

// this class handles atomic updates and memory synchronization
final AtomicReference<ScheduledFuture> futureReference =
    new AtomicReference<ScheduledFuture>();
ScheduledFuture exec = executor.scheduleAtFixedRate(
    new Runnable() {
        int totalSends = 0;
        public void run() {
            //do stuff here
            if (++totalSends >= repetitions) {
                // we need to wait for the future to be initialized
                while (true) {
                    ScheduledFuture future = futureReference.get();
                    if (future != null) {
                        future.cancel(true);
                        break;
                    }
                    try {
                        Thread.sleep(1);
                    } catch (InterruptedException e) {
                        Thread.currentThread.().interrupt();
                    }
                }
            }
        }
    },
    0, delay, TimeUnit.MILLISECONDS);
// this sets the future reference so the thread can use it
futureReference.set(exec);

Answer 2

There are basically two ways I know of to get around this. Unfortunately neither of them is especially nice regarding the resulting code quality, so I'm not sure if I can recommend them.

The first solution would be to make exec a final one-element array. Then you can assign exec[0] = something after declaration even tho the array itself is final. A variation of this is to use/create some reference class (since you can alter attributes of final references, but not the references themselves). The following is a simple example, but keep in mind that it does not take any concurrency issues into consideration (see further down):

    final ScheduledFuture[] exec = new ScheduledFixture[1];
    exec[0] = executor.scheduleAtFixedRate( //<< initialized on this line
            new Runnable(){
                int totalSends = 0;
                public void run(){
                    //do stuff here

                    if(++totalSends >= repetitions) exec[0].cancel(true); //<< here is says exec might not be initialized
                }
            },
        0, delay, TimeUnit.MILLISECONDS);

Alternatively you could move exec out of the local scope of the method and make it a class attribute instead.

I must however warn you that, especially with the initial delay of zero, there is a real possibility of the code inside the runnable being executed before the scheduleAtFixedRate method returns, in which case exec[0] will still be null. Also, you should use synchronization to ensure that the value of exec[0] as set by the main thread will be available to the thread responsible for executing the runnable.

Both of the above solutions should work, but I don't think that either of them is especially nice.