Boolean not changing in thread

Question

I have a class MPClient and MultiplayerMatch. MultiplayerMatch, in his constructor, creates a MPClient runnable thread.

To avoid data overflow, I have a boolean named "moved" in MultiplayerMatch that changes to true when the player is moving.

In the updateMatch method, if there's any player movement, "moved" changes to true, which allow MPClient to enter an if statment (inside while). This way MPClient only sends data to the server when something changes on the game.

Neverthless, when the flag is true, in MPClient that change is not registed! MPClient still "thinks" moved equals false, even after that flag changed in MultiplayerMatch, and as a consequence, nothing is sent to the server...

After a few tests, I noticed that if I run it in Debug Mode, since I have some breakpoints, that change is registered and everything works great! Why is the boolean change only "seen" though Debug Mode? Does it have something to do with the app "running speed", since there are breakpoints?

Here's only the important part of the code:

MPClient:

public class MPClient {
static final int TIME_OUT = 5000;
Client client;
MultiPlayMatch match;

public MPClient(String name, int team, MultiPlayMatch match) {
    this.match = match;
    client = new Client();
    client.start();

    Network.registerPackets(client);
    addListeners();

    try {
        client.connect(TIME_OUT, "127.0.0.1", Network.PORT);
    } catch (IOException e) {
        e.printStackTrace();
        client.stop();
    }

    /*this comment is just to show that here is the place where the login information is sent to the server, instead of showing all the code*/

    PlayerInfo playerInfo = new PlayerInfo();
    Network.UpdatePlayer updatePlayer = new Network.UpdatePlayer();
    updatePlayer.name = name;
    updatePlayer.team = team;

    while(true) {
        if(match.moved) {       //--> this is the variable that is always false
            playerInfo.x = match.getClientPlayerX(team);
            playerInfo.y = match.getClientPlayerY(team);

            updatePlayer.x = playerInfo.x;
            updatePlayer.y = playerInfo.y;
            client.sendTCP(updatePlayer);
            match.moved = false;
        }
    }

}

private void addListeners() {
    client.addListener(new Listener.ThreadedListener(new Listener() {
        @Override
        public void received(Connection connection, Object object) {
            if(object instanceof Network.UpdatePlayer) {
                Network.UpdatePlayer updatePlayer = (Network.UpdatePlayer) object;
                match.setPlayerPosition(updatePlayer.x, updatePlayer.y, updatePlayer.name, updatePlayer.team);
            }
        }
    }));
}
}

MultiplayerMatch:

public class MultiPlayMatch extends Match {

public boolean moved;

public MultiPlayMatch(){
    super(0);

    Random r = new Random();
    int aux = r.nextInt(2);
    aux = 0;
    if(aux == 0){
        homeTeam = new Team("Benfica", Team.TeamState.Attacking, w);
        visitorTeam = new Team("Porto", Team.TeamState.Defending, w);
    } else{
        homeTeam = new Team("Benfica", Team.TeamState.Defending, w);
        visitorTeam = new Team("Porto", Team.TeamState.Attacking, w);
    }
    //homeTeam.controlPlayer(0);

    numberOfPlayers = 0;
    moved = false;
}

@Override
public void updateMatch(float x, float y, Rain rain, float dt) {
    homeTeam.updateControlledPlayerOnline(x, y);

    rain.update();
    w.step(Constants.GAME_SIMULATION_SPEED, 6, 2);

    if(x != 0 || y != 0) moved = true;      //this is the place the variable is changed, but if it isn't in debug mode, MPClient thinks it's always false
}

public void setPlayerPosition(float x, float y, String name, int team) {
    if(team == 0)
        homeTeam.changePlayerPosition(x, y, name);
    else
        visitorTeam.changePlayerPosition(x, y, name);
}
}

Answer 1

tl;dr

AtomicBoolean is a convenient alternative to volatile.

This class wraps and protects a nested primitive boolean value while ensuring proper visibility.

Instantiate:

public final AtomicBoolean moved = new AtomicBoolean( false ) ;

Getter:

boolean x = moved.get()  // Returns current value. 

Setter:

moved.set( false ) // Sets a new value.

Get, then set:

boolean x = moved.getAndSet( false ) ;  // Retrieves the old value before setting a new value. 

AtomicBoolean

The Answer by agamagarwal is correct. You have fallen into the visibility conundrum that occurs when accessing variables across threads. One solution is the use of volatile shown there.

Another solution is the Atomic… classes bundled with Java. In this case, AtomicBoolean.

The Atomic… classes wrap a value, and add thread-safe methods for accessing and setting that value.

I often prefer using the Atomic… classes rather than volatile. One reason for this preference is that it makes quite clear and obvious to the user that we are using a protected resource across threads.

Instantiation:

public class MultiPlayMatch extends Match {

public final AtomicBoolean moved = new AtomicBoolean( false ) ;
…

Notice two things about that instantiation:

• final ensures that we do not swap out one AtomicBoolean object for another. Such swapping would put us right back into the variable visibility conundrum we are trying to escape.
• The AtomicBoolean object is being instantiated at the same time as this outer object (MultiPlayMatch in your case) is being instantiated. So we have ensured that an instance of AtomicBoolean exists before any access, including any access across threads. If we waited until later (“lazy” loading), then we would be falling back into that variable visibility conundrum we are trying to escape.

Getting the value:

if ( this.match.moved.get() ) { …  // Returns the primitive `true` or `false` value wrapped within this `AtomicBoolean` object. 

And setting the value:

this.match.moved.set( false ) ;

You may want to get the current value while also setting a value in an immediate thread-safe “atomic” (combined) operation:

boolean oldValue = this.match.moved.getAndSet( false ) ;

---

To learn all about concurrency in Java, see the book, Java Concurrency in Practice by Brian Goetz, et al.

Answer 2

volatile

This is because it is reading a cached value of match.moved variable instead of the latest. To avoid this, declare the variable as volatile

public volatile boolean moved;

Read more here