Java 7: IF condition triggered when false

Question

I have this piece of code:

void timerCountDown(){
    while(RaftNode.getTimeoutVar()){
        long x = System.currentTimeMillis();
        if(x >= RaftNode.limit){
            System.out.println(x);
            System.out.println(RaftNode.limit + " THIS SHOULD BE LESS THAN");
            System.out.println(System.currentTimeMillis() + " THIS");
            System.out.println("TIMED OUT");
            raft.RaftNode.setTimeoutVar(false);
            nextRandomTimeOut();
            raft.RaftNode.onTimeOut();
        }
    }
}

So basically, this is a time-out function and the time-out is refreshed by another condition. My issue is that the x >= RaftNode.limit condition keeps triggering even though it is false (through the print statements).

My console outputs:

1431532870542
1431532872508 THIS SHOULD BE LESS THAN
1431532870542 THIS

So x is indeed the current time, but even though it is less than the limit, the condition is being triggered.

I have no idea why!

The limit var is

public static long limit;

so nothing fancy here

Something is probably changing the limit between the comparison and the print statement — Kon, May 13 '15 at 16:15
One thing I would note is you're calling `System.currentTimeMillis()` a second time. Get rid of your third print statement because it's confusing, you should be printing x there. Granted this code executes fast enough that `System.currentTimeMillis()` is the same as the value you stored in x, but that's not guaranteed — StormeHawke, May 13 '15 at 16:22
@Kon - The way it's written, the limit will only ever be increased, so I don't think that would make a difference — dan, May 13 '15 at 16:23
@StormeHawke - originally there was no x, I was just calling System.currentTimeMillis() on its own. The x was just for debugging purposes — dan, May 13 '15 at 16:24
Save RaftNode.limit to a temp variable before enter the condition and check if still prints a larger number for the temp — Rodrigo López, May 13 '15 at 16:24
Are you using threads? The update of the values of a shared variable can be delayed. Make `limit` volatile or so. — Joop Eggen, May 13 '15 at 16:24
See http://stackoverflow.com/help/mcve. Please update with a minimal example which reproduces the behavior you are seeing. — Radiodef, May 13 '15 at 16:26
As @JoopEggen suggested, try changing `public static long limit;` to `public voliatile static long limit;` — StormeHawke, May 13 '15 at 16:26
Make the limit a `public static ` **final** `long limit;`. If the compiler then complains somewhere, this "somewhere" might lead you to the answer. — Marco13, May 13 '15 at 16:27
@Marco13 That's a veteran developer suggestion, I like it. Make the compiler do the work. — Kon, May 13 '15 at 16:27
@user3601503, I see you fixed your issue but it would have been better for debuggin to print the values before and after the `if` condition. It would have shown to you that value is changing between the calls. — akostadinov, May 13 '15 at 16:32
@akostadinov I actually originally tried this, but when I did the bug went away - I'm not too knowledgeable on why this happened, perhaps printing the value forced it to update? — dan, May 13 '15 at 16:35
@JoopEggen Is this a threading issue or JVM optimizer issue? Maybe the the If-stmt remembered a lower value, because JVM optimizer "inlined" it? — Mecon, May 13 '15 at 16:37
@user3601503 are you in multi threaded env? Would you have gotten the error in single threaded mode as well? — Mecon, May 13 '15 at 16:39
@Mecon I was in a multi threaded env, several threads were able to update the var at once. I wouldn't have been able to complete the task with a single threaded mode so I am not sure! — dan, May 13 '15 at 16:43
It is basically a threading issue, a search on java `volatile` will point further. Rather silly but threads can have stale values of some variable altered by another thread. With `volatile` the compiler ensures with extra code that threads get updated values. That variable values are copied into a thread might be considered an optimization issue. But then look at JIT too, LL2 cache and so on. — Joop Eggen, May 13 '15 at 16:51
@user3601503 Since this is a multi threaded env, I would suggest using "AtomicLong" instead of " volatile long". http://stackoverflow.com/questions/12859008/what-is-the-difference-between-using-a-volatile-primitive-over-atomic-variables — Mecon, May 13 '15 at 16:53
@Joop you should post that as an answer so the OP can mark it and this question can be considered "answered" — StormeHawke, May 13 '15 at 17:12

score 1 · Answer 1 · answered May 13 '15 at 16:29

Could you try something like the following?

void timerCountDown(){
    long limit = RaftNode.limit;
    long raft_time = RaftNode.getTimeoutVar();
    long x = System.currentTimeMillis();

    while(raft_time){
        x = System.currentTimeMillis();
        if(x >= limit){
            System.out.println(x);
            System.out.println(limit + " THIS SHOULD BE LESS THAN");
            System.out.println(x + " THIS");
            System.out.println("TIMED OUT");
            raft.RaftNode.setTimeoutVar(false);
            nextRandomTimeOut();
            raft.RaftNode.onTimeOut();
        }
        raft_time=RaftNode.getTimeoutVar();
    }
}

score 1 · Accepted Answer · answered May 13 '15 at 19:47

It can only be a problem using threads. The update of the values of a shared variable can be delayed. Make limit volatile or so.

It is basically a threading issue, a search on java volatile will point further. Rather silly but threads can have stale values of some variable altered by another thread. With volatile the compiler ensures with extra code that threads get updated values. That variable values are copied into a thread might be considered an optimization issue. But then look at JIT too, LL2 cache and so on.

(started as comments.)

Java 7: IF condition triggered when false

2 Answers2