ServerSocket in Java hangs thread, unlocks on placing a break point?

Question

I seem to have an unusual problem that I can't understand the root cause to.

I am using a ServerSocket to handle connections to a server I'm writing. The ServerSocket accepts connections in it's own thread, and can be controlled from the main thread via isAccepting and isActive variables I set up.

What should happens: Server starts and is accepting connections (via putty). I use a command to close the server socket. The socket closes and the thread idles (I notice this causes a SocketException that I catch). I use a command to open a new server socket and it accepts connections again. I'm able to connect and can exit the application via a command that shuts down the socket and exits the loop accepting connections

What happens:

Server starts and is accepting connections (via putty). I use a command to close the server socket. The socket closes and the thread idles(I notice this causes a SocketException that I catch). I use a command to open a new server socket and that's where the thread hangs. It does not print out any debug info that's in the code, nor does it responde to opening/closing the ServerSocket. using the Exit command hangs the application on the exit routine. Funny thing is, if I set a breakpoint anywhere in the thread code, it unstucks and completes, exiting.

TL;DR - closing the socket jams the thread until I place a breakpoint, after which the code executes normally.

Tried exporting into an executable JAR and the application hangs on exit, just like in Eclipse.

Relevant parts of code below:

public class ConnectionManager extends Thread implements IEverfreeManager {

private final int defaultPort = 8002;

private boolean isAccepting = true;
private boolean isActive = true;

private static ConnectionManager instance;

private ServerSocket serverSocket;
private int portNumber = defaultPort;

private Socket workSocket;

public static ConnectionManager instance(){
    if (instance == null)
        instance = new ConnectionManager();
    return instance;
}

public ConnectionManager() {

}

public boolean isAccepting() {
    return isAccepting;
}

public void setAccepting(boolean isAccepting) {
    this.isAccepting = isAccepting;
    try{
        if (!isAccepting && !serverSocket.isClosed()){              
             serverSocket.close();
             System.out.println("Closed server on port "+portNumber);
        } else{
            serverSocket = new ServerSocket(portNumber);
            System.out.println("Server on port "+portNumber+" is now accepting connections");
        }
    }catch(Exception e){
        System.out.println("failed to stop accepting");
        e.printStackTrace();
    }

}

public boolean isActive() {
    return isActive || isAlive();
}

public void setActive(boolean isActive) {
    this.setAccepting(isActive);
    this.isActive = isActive;

}

public int getPortNumber() {
    return portNumber;
}

public void setPortNumber(int portNumber) {
    this.portNumber = portNumber;
}



private int getNewConnectionId(){
    return ++connectionIdCounter;
}

@Override
public void run() {
    super.run();


    try {       
          System.out.println("Starting up Connection Manager");
          System.out.println("Starting server on port "+portNumber);
        serverSocket = new ServerSocket(portNumber);
        System.out.println("Server running and ready to accept players");
        while (isActive){
            if (isAccepting){
                try{
                    System.out.println("Waiting for connection...");
                    workSocket = serverSocket.accept();
                    System.out.println("Connected with "+workSocket.getInetAddress());
                    int id = getNewConnectionId();

                } catch (SocketException e){
                    System.out.println("Notice: "+e.getMessage());
                }

            }
        }

    }catch(Exception e) {

        e.printStackTrace();

    }

}

@Override
public void closeManager() {
    setActive(false);       
}

Using setAccepting(false) and then setAccepting(true) doesn't produce the

System.out.println("Waiting for connection...");

message until I put a breakpoint in the code.

Using closeManager() after setAccepting(false) produces the same results.

Using just closeManager() without touching setAccepting() exits gracefully (despite having the procedure activated during shutdown)

Any insight would be very appreciated

Answer 1

There's nothing thread-safe in this class. There are very fundamental problems with almost every function.

Both isAccepting and isActive both need to be either volatile or be modified in a synchronized manner to be thread-safe. If another thread is calling functions that mutate these fields and you have your run method already looping over them you may get unpredictable results. Attempting to view boolean flags that have no memory visibility guarantees is always a bad idea.

setAccepting() has a race condition where your run() thread may attempt to listen on a socket that is immediately about to be closed.

The singleton ConnectionMananger instance could have multiples be created. In your case your constructor does nothing but it's generally safer to not have to create instances. Use double checked locking to implement this so only one instance will ever be created.

Your immediate problem could likely be 'fixed' by making both the is* member fields volatile but like I said you still have too many other issues in this class that it would be complete safe to use in a multithreaded environment. In addition, catching Exception and simply printing is usually wrong. And you usually want to subclass runnable and pass that to the thread constructor rather than creating a subclass of thread.