Code doesn't work unless I include a println or breakpoint

Question

I'm making a rudimentary multiplayer shooter as part of a university course on computer graphics.

For client->server communication I'm using UDP. Each client has a ClientController thread, which handles user input and sends it via socket to the server. The server is running a ServerClientManager, which processes the messages, and updates the player's object in the gamestate.

Here's the code for the ClientController:

/**
 * A class that handles communication from client to server. 
 */
public class ClientController implements Runnable {
    private String serverAddress;
    int serverPort;
    Integer clientId;
    @SuppressWarnings("unused")
    private boolean forward, backward, left, right, fire, up, down; 

    public ClientController (String serverAddress, int serverPort) {
        System.out.println("ClientController started");
        this.serverAddress = serverAddress; 
        this.serverPort = serverPort; 
        clientId = null; 
        reset(); 
    }

    public synchronized void forward() {
        forward = true; 
    }
    ...

    private synchronized void reset() {
        forward = false; 
        backward = false; 
        left = false; 
        right = false; 
        fire = false; 
        up = false; 
        down = false; 
    }

    @Override
    public void run() { 
        System.out.println("ClientController thread is running");
        while (true) {
            //System.out.println("tick");
            if (clientId != null) {         
                try {
                    ClientUpdate update = new ClientUpdate(forward, backward, left, right, fire, up, down, clientId); 
                    ClientUDPSender sender = new ClientUDPSender(serverAddress, 1233, update.toJson());
                    Thread worker = new Thread(sender); 
                    worker.start(); 
                    Thread.sleep(15);
                    reset(); 
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        }
    }

    public synchronized void setClientId(int clientId) {
        this.clientId = clientId; 
    }
}

This sends a update to the server. On startup, the client starts this thread. The client then separately receives it's clientId via TCP in another class (Initial gamestate and connection info is sent via TCP). Once the clientId has been set, the UDP client starts communicating.

The bizarre thing is that none of this works unless I do one of two things:

1) Uncomment the System.out.println("tick") line.

2) Put a breakpoint inside the try statement and then resume from there.

If I simply run the program as-is, it never reaches the inside of the try statement. If I include the println or breakpoint, it runs just fine, and the update propagates through to the server as intended.

I'm quite baffled.

Answer 1

The bizarre thing is that none of this works unless I do one of two things:

1) Uncomment the System.out.println("tick") line.

PrintStream.println(...) is a synchronized method so I suspect that your problems are with memory synchronization.

The client then separately receives it's clientId via TCP in another class

If I'm understanding, it seems like clientId is being set by another thread. If this is true then it must be marked as volatile otherwise the spinning thread will not see the update. Even though the setClient(...) method is synchronized there is nothing within your while(true) loop which crosses a memory barrier and updates the clientId value in the spinning thread's memory cache. Once you mark the clientId as volatile you can remove the synchronized keyword on the setter.

volatile Integer clientId;

Also, spinning is pretty gross. Maybe put in a Thread.sleep(10) or something to slow it down a bit?