Java network code not sending

Question

I'm writing my first non-trivial Java app that uses:

• networking
• a GUI
• threads

It's a IM program. When I send a message, the server doesn't output what it should. I'm sorry this description is so bad, but I don't know how to narrow the problem down further.

public class MachatServer {
     //snip
     public static void sendMessage(int targetId, int fromId, String message) {
         ConnectedClient targetClient = getClient(targetId);
         // Also runs
         System.out.println("Sending message: " + message + "\n\nfrom " + fromId +  " to " + targetId);
         targetClient.addOutCommand("/message:" + fromId + ":" + message + "\n");
     }
}
class ConnectedClient implements Runnable {
public void run() {
    String contact;
    contact = s.getInetAddress().toString();
    System.out.println("Connected to " + contact);
    try {
        out.write("/connected" + "\n");
        out.flush();
        String command;
        while(true) {
            if(shouldExit) {
                s.close();
                break;
            }
            if(in.hasNextLine()) {
                command = in.nextLine();
                commandProcessor.addInCommand(command);
            }
            Thread.sleep(100);
         }
    } catch(Exception e) {
        e.printStackTrace();
    }
}
// snip
public void addOutCommand(String command) {
    commandProcessor.addOutCommand(command);
    //
    // My guess is that the problem is with this method as the next line
    // Does not print out.
    //
    //
    System.out.println("" + thisId + " recieved to send: " + command);
}
}
class CommandProcessor implements Runnable {  
// snip  
public void run() {
    String currentCommandIn;
    String currentCommandOut;
    while(true) {
        try {
            currentCommandIn = inQueue.poll();
            if(currentCommandIn != null) {
                System.out.println("Processing: " + currentCommandIn);
                String[] commandArr = CommandParser.parseRecievedCommand(currentCommandIn);
                if(commandArr[0].equalsIgnoreCase("message")) {
                    int target = Integer.parseInt(commandArr[1]);
                    String message = commandArr[2];
                    // This definetly runs
                    System.out.println("Message sending to: " + target);
                    MachatServer.sendMessage(target, this.conId, message);
                } else if(commandArr[0].equalsIgnoreCase("quit")) {
                    // Tell the server to disconnect us.
                    MachatServer.disconnect(conId);
                    break;
                }
            currentCommandOut = outQueue.poll();
            if(currentCommandOut != null) {
                try {
                    out.write(currentCommandOut + "\n");
                    System.out.println(currentCommandOut + "sent");
                    out.flush();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
      } catch(Exception e) {
            e.printStackTrace();
      }    
    public synchronized void addOutCommand(String command) {
    if(command != null) {
     try {
      outQueue.push(command);
     } catch(Exception e) {
      System.out.println(command);
      e.printStackTrace();
     }
        // Does not print
        System.out.println("Ready to send: " + command);
    } else {
        System.out.println("Null command recieved");
    }
    //snip
}

The full source code is at my github, in case I have narrowed the problem down incorrectly.

The expected output should be when I telnet in and send "/message:0:test", it should send "/message:myid:test" to the client with ID 0. The actual output is nothing.

Answer 1

Maybe the problem is that the data you send is to short....

A friend of mine had a similar problem a couple of years ago, and it turned out the data was being buffered until it had enough data to send...

It had something to do with optimizing the amount of network traffic... I believe he mentioned something called "Nagle's algorithm" when he finally solved it....

Hope this can be of some help...

Answer 2

This is probably not a complete answer, but there are a few serious issues with your code that could be the cause of your problem, so you should fix those first.

First, the loop in CommandProcessor.run is busy-waiting, i.e., it runs constantly. You should use blocking operations. Also, inQueue and outQueue are accessed from two different threads so you need synchronization on every access. I recommend using something implementing the java.util.concurrent.BlockingQueue interface to solve both issues. And finally, when checking your full code, it appears that you also need to synchronize access to the ConnedtedClient.shouldExit field (I believe you can use `java.util.concurrent.atomic.AtomicBoolean as a replacement but I'm not sure).

And the reason why this could be the cause of your problem: Since CommandProcessor.run is not synchronizing on anything (or accessing anything volatile), the Java virtual machine can assume that nothing from outside can modify anything it examines, so in theory, when the run method first notices that inQueue and outQueue are both empty, it can optimize the whole method into nothing, as it can assume that it is the only thing that can modify them. But I don't know whether this can actually happen in practice, as the JVM needs to know quite a bit about the LinkedList implementation and notice that the thread is just doing these two checks in a loop. But it's always best to be safe because that way your code is guaranteed to work.

Answer 3

The field outQueue is uninitialized in CommandProcessor, and you commented out the printStackTrace() that would have helped you figure it out.