Are while true loops that check only for one condition thread safe?

Question

I'm using a while loop that is checking for a condition to become true:

while(Game.gameState != Game.CLOSING)
{
    if(reconnecting)
    {
        time = System.currentTimeMillis();
    }
    else
    {
        while(time + desiredWait > System.currentTimeMillis())
        {
            try{Thread.sleep(5);}catch(InterruptedException e){}
        }

        time += desiredWait;
    }

    synchronized (Game.gameStateLock)//added to circumvent the problem
    {
        if(Game.gameState == Game.INPROGRESS)//problematic condition
        {
            while(true)
            {
                synchronized (Lockstep.nextTurns)
                {
                    if(!Lockstep.nextTurns.isEmpty() || lockstep.currentTurn == 0)
                        break;
                }

                try{Thread.sleep(5);}catch(InterruptedException e){}

                time = System.currentTimeMillis();
            }

            Lockstep.attemptTurn();
        }
    }
}

The loop runs uninterrupted without sleeping (if reconnecting is true), and therefore accesses Game.gameState (of type int) two times in each cycle. If gameState is now changed in a second thread (e.g. via network), the while loop/if condition doesn't detect it and continues refusing to execute the code in the if block even if it should. The added synchronized(Game.gameStateLock) solves this problem. It is also really hard to debug, cause printing the gameState or anything else will cause the problem to be nonexistent. My guess is that I/O interrupts/sleeps the thread or causes it to write/read that much data, that the cache of the cpu is cleared and the gameState variable which has been read from cache the whole time has to be reloaded from the RAM.

Can this be the reason? I assumed primitive data types to be not that much of a problem when dealing with multiple threads (unless you're checking a boolean and then setting it to block other threads). Are primitive data types in Java thread safe? (i can't even synchronize on them, needed a dummy object)

Does the problem evolve in any way by making gameState volatile? — Arthur Noseda, Aug 01 '16 at 16:28
If gameState is not defined as volatile, then you will have a problem with your outer while loop. You could be accessing its value while it could be set at the same time by an external thread. See http://stackoverflow.com/questions/9278764 — mdewit, Aug 01 '16 at 16:34
Adding volatile will prevent you from getting a ConcurrentModificationException. You must also make sure that you only synchronise on an object that is marked as final and also that you use synchronisation correctly in your other classes. — mdewit, Aug 01 '16 at 16:42
@mdewit - CME has nothing to do with this question whatsoever. — jtahlborn, Aug 01 '16 at 16:45
Re, "i can't even synchronize on them,..." It would be a mistake to think that you need to synchronize on the data that you are trying to protect. In fact there are good reasons to synchronize on a _private_ dummy object when the thing that you want to protect is a publicly visible object. — Solomon Slow, Aug 01 '16 at 17:10
@jtahlborn I commented on an issue I saw in his code. And I feel its important that he should fix it. Hench why it is a comment and not an answer. — mdewit, Aug 02 '16 at 07:04
@mdewit - unless the code has changed from that which you were originally commenting on, my comment still applies. CME has nothing to do with the code in question (nor the overall question itself). — jtahlborn, Aug 02 '16 at 18:45

score 2 · Accepted Answer · answered Aug 01 '16 at 16:41

"thread safe" is not really the term you are looking for, which may be why you are struggling. your code is technically "thread safe", in that you won't encounter any corruption due to multi-threading. what you are missing however, are guarantees related to "visibility". in your situation, there is no guarantee that the change made by one thread will be "visible" to another. there are a variety of ways to make changes visible. in your situation, making gameState volatile would be sufficient to force cross-thread visibility of changes to that value.

score -1 · Answer 2 · answered Aug 01 '16 at 16:35

-1

Primitive types are not threadsafe! Check this: http://i-proving.com/2009/04/23/java-primitive-thread-safety/?showComments=true

answered Aug 01 '16 at 16:35

beeb

1,615
1
12
24

this isn't an issue of thread safety, it's a question of visibility of changes. – jtahlborn Aug 01 '16 at 16:40
the provided link addresses issues related to atomicity of updating a value, which isn't really the issue here. – jtahlborn Aug 01 '16 at 16:43

Are while true loops that check only for one condition thread safe?

2 Answers2