Java Simple Concurrent Server

Question

I must design a simple server which receives messages from multiple nodes and stores them in a message repository.

The code for the server is :

    public class CommunicationServer implements Runnable {
      private List<String> messages;
      private MessageRepository messageRepository;
      private boolean serverBusy;

      public CommunicationServer() {
       messages = new ArrayList<String>();
       messageRepository = new MessageRepository();
       serverBusy = false;
     }

      @Override
      public void run() {
      try {
       while (!Thread.interrupted()) {
        synchronized (this) {
          if (messages.size() > 10) {
            serverBusy = true;
            addMessageToRepository();
            notifyAll();
          }
          else {
            serverBusy = false;
            wait();
          }
        }
      }
    }
    catch (InterruptedException e) {
      System.out.println(e.getMessage());
     }
    }

    public synchronized void receiveMessage(String message) {
     if (messages.size() < 10) {
      messages.add(message);
    }
    }

  private void addMessageToRepository() {
    if (messages.size() != 0) {
      messageRepository.addMessage(messages.remove(0));
    }
  }

  public void showMessageRepository() {
    messageRepository.showStoredMessages();
  }

  public synchronized boolean isServerBusy() {
    return serverBusy;
  }
}

The code for the node is:

public class Node implements Runnable {
  private static final String CHARACTERS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVXYZ0123456789";
  private static final int MESSAGE_LENGHT = 5;
  private Random random = new Random();
  private CommunicationServer communicationServer;

  public Node(CommunicationServer communicationServer) {
    this.communicationServer = communicationServer;
  }

  @Override
  public void run() {
    try {
      while (!Thread.interrupted()) {
        while (communicationServer.isServerBusy()) {
          wait();
        }
        communicationServer.receiveMessage(generateRandomString());
      }
    }
    catch (InterruptedException e) {
      System.out.println(e.getMessage());
    }
  }

  private String generateRandomString() {
    StringBuffer randomMessage = new StringBuffer();

    for (int i = 0; i < MESSAGE_LENGHT; i++) {
      randomMessage.append(CHARACTERS.charAt(random.nextInt(51)));
    }
    return randomMessage.toString();
  }
}

In main I just create a thread for the server and 5 threads for the nodes and let them run for some time. The server sleeps until it receives 10 messages, after that it must wake up to process the messages. The problem is I can't figure it out where to call notifyAll() in order to wake the thread responsible for the server.

Answer 1

Making CommunicationServer implements Runnable doesn't really make sense and exposes your basic misunderstanding: you identify the actors in your process (server, client) with threads. A thread is not the actor; a thread is where actor's code gets executed.

So, when in your CommunicationServer you say wait(), you don't make the server wait for messages; you make that particular thread wait on the server object as its monitor. Likewise, then you say notifyAll(), you are not "notifying all servers"; you are notifying all threads waiting on that particular monitor. It should be some code in the client which notifies the threads that are currently waiting on the server's monitor, and some code in the server which notifies those waiting on the client monitor.

As a general rule of thumb, when you find yourself using both wait() and notify() within the same synchronized block, you can be pretty sure there's something wrong with your logic.