0

I am working on a server/client based game using the KryoNet java library as well as slick. When the server class receives a connection from a client, it sends necessary startup information to the client, including what player number it is. One receiving this, the client starts slick and begins operating normally. The code for this is:

    boolean started = false;
    while(!started){
        System.out.println(cs.playerNum);
        if(cs.playerNum != -1){
            cs.startSlick();
            started = true;
        }
    }

The playerNum is set by another thread when the value is received from the server. For a while I could not get this to work (cs.startSlick() was never called), and eventually I got frustrated and began logging playerNum each time the loop ran. By adding System.out.println(cs.playerNum), the code began working, the loop would evaluate properly and slick would be started.

How is it possible that System.out.println does this? I have tried replacing it with other functions, and even other functions which take cs.playerNum as a parameter, but only when I specifically print cs.playerNum can I get the loop to work. If I need to include more source I can, but the issue seems to be directly here since I have tried replacing System.out.println with other functions to no success.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
htuy42
  • 307
  • 3
  • 10
  • Any threading going down? System.out.println would cause the current thread to be suspended allowing other threads to do there thing. – Michael Lloyd Lee mlk Sep 30 '15 at 15:47
  • 1
    dont know the answer to your problem, but this in general is very bad code! if a thread has to wait for another thread, use wait() and notify()! busy waiting is never a good choice and can cause weird behavior like yours! – Michael Sep 30 '15 at 15:49
  • To be fair I have not worked with multithreaded aplications before, so its entirely possible I'm doing something very wrong. I will look at using what you have suggested, but I am curious still as to what is happening. – htuy42 Sep 30 '15 at 15:59
  • @mlk that might be it. If another thread is started, and allowed to run on its own, would it actually not do anything while this thread was running? Also, I have tried sleep() ing this thread, and that does not solve the problem: would that do the same thing? Sorry I'm a little confused, have not worked with threads much before. – htuy42 Sep 30 '15 at 16:02
  • 3
    What is happening: The other thread modifies the `started` variable, but this modification is not visible for this thread. (The reasons are buried in caching and the Java memory model). It **could** already be solved by declaring the `started` variable as being `volatile` (to make sure that every thread sees the changes), **BUT** what you are doing there is called "busy waiting", and a **very** bad practice unless your goal is to use your CPU to keep your coffee hot and steamy. You should have a look at `java.util.concurrent`, and maybe solve it with a `CountDownLatch` or so... – Marco13 Sep 30 '15 at 16:02

2 Answers2

3

You kind of answered your own question when you said "The playerNum is set by another thread". What you have is a classic race condition. If your code is able to execute quickly enough, then that playerNum will not have been set by the time it is needed. However, if something were to delay or "interrupt" your code, then the other thread will have time to set the playerNum value, and your code will work as you expected.

A system call to perform IO forcibly suspends a thread while it waits for that IO action to occur. This happens when you call System.out.println which causes your seemingly tight code to pause briefly yielding to the other thread and allowing you to then retrieve the desired value.

This is a very basic threading problem, and you will run into much more complex threading problems writing threaded code. As such I would definitely suggest you spend some time reading up on threading in general and understanding how synchronized functions work as well as wait() and notify(), as was suggested in the comments.

AaronM
  • 2,339
  • 2
  • 17
  • 18
  • Thanks. I did kind of just jump in, I suppose I will take some time to learn more. Any especially good suggestions as to where to start, or should I just hit up google? – htuy42 Sep 30 '15 at 16:04
  • 1
    This is not what I would call a "race condition" (although I don't want to argue about wordings here). It is simply a visibility issue, implied by the Java Memory model. – Marco13 Sep 30 '15 at 16:04
  • @Marco13, Seems like you do want to argue or you would not have left a comment.. Maybe Google is wrong as well, but the answer to "Race Condition" - A race condition is an undesirable situation that occurs when a device or system attempts to perform two or more operations at the same time, but because of the nature of the device or system, the operations must be done in the proper sequence to be done correctly. – AaronM Sep 30 '15 at 16:06
  • 1
    @htuy42 Nothing wrong with jumping in. In my opinion though, threading is one of the hardest things that the typical programmer will ever encounter, and there are definitely a lot of gotchas. Stick with it though and it will start to click. – AaronM Sep 30 '15 at 16:08
  • @AaronM The sequence is: 1. The variable has to be changed. 2. The variable has to be checked (maaany times here...). Everthing is fine in terms of the *sequence*. The fact that the change is not *visible* to the other thread is unrelated to that. It was mainly a nitpick, because I think that an answer (that is accepted) should be worded carefully to not suggest the wrong thing. – Marco13 Sep 30 '15 at 16:10
  • @Marco13 I think I see what you are saying, but don't think you see what I am saying. The sequence that I am suggesting is that eventually that thread will yield and then the change will become visible. I think that still qualifies as a race condition. There are many situations where a race condition will eventually resolve itself, but with unexpected or undesired results. Are you saying that variable will never become visible? – AaronM Sep 30 '15 at 17:18
  • @AaronM The JVM is allowed to rewrite `while (!started) { ... }` into `if (!started) while (true) { ... }`, so the value will never become visible. – Boann Sep 30 '15 at 17:26
  • @Boann Now I am definitely not going to claim to understand the JVM at that level, and find it very difficult to believe that the JVM rewrites anything, but would understand if javac did some rewriting during generation of bytecode. But I cannot see how what you state could be true. That change most certainly changes the meaning and flow of the loop and would break things in situations where other threads were not involved. Perhaps if (!started) while (!started) { . . .} which does not change things. The JVM doesn't have enough information to make the change you suggest. – AaronM Sep 30 '15 at 17:33
  • @AaronM Sorry! I misread the code, that rewrite would be wrong in this case, but the JVM does rearrange code significantly during optimization. The JVM can see that `cs.playerNum` doesn't change on the current thread while waiting/looping, and it can assume no other thread changes it either, which means it caches the value, so the loop becomes infinite. Calling `System.out.println` defeats such optimizations because it's a `synchronized` method that creates a [happens-before](https://docs.oracle.com/javase/tutorial/essential/concurrency/memconsist.html) relationship between threads calling it. – Boann Sep 30 '15 at 17:57
  • @Boann I understand that compilers can and may make these type of optimizations. However, I do not think that the compiler can in this case at least with the context given. We do not know the initial value of playerNum, therefore we do not know it is not -1. If it is not -1 another method gets executed, that method has the ability to change the value of playerNum, and therefore the compiler cannot make the optimization that you have suggested. If we had more context perhaps that assumption could be made. If we knew that playerNum was set to -1 immediately before this loop, that changes. – AaronM Oct 01 '15 at 22:05
2

That sleeping doesn't fix the problem indicates this is not just about giving the thread time to work, it's about memory visibility across threads. When your thread calls println it acquires a lock on the console, which forces changes to cs.playerNum become visible.

You don't say how you update playerNum or whether it's volatile, but it seems like you're seeing an optimization where the JVM is not aware it needs to make this thread aware of updates to playerNum. The JVM can make optimizations like reordering bytecode or caching values, and only knows not to do that if your code indicates that it's not allowed by doing things like making variables volatile or doing locking.

Busy waiting should be avoided, this really needs to be replaced with waiting for a notification, or reading from a queue, or otherwise using some higher-level construct from java.util.concurrent.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276