3

i'm stuck with this code, and i think that i'm missing something really important. when the thread is running i can add message to the vector but when it calls notify it seems that the getNextMessageFromQueue() keep to stay on the wait.

Am i locking the messages vars?

Thanks for your help.

My dispatcher class which send all messages to my clients :

private Vector<Message> messages = new Vector<Message>(); 

public synchronized void addMessage(Message message) {
    messages.add(message);
    notify(); 
}

private synchronized Message getNextMessageFromQueue() throws InterruptedException { 
    while (messages.size() < 1) {
        wait(); 
    }

    Message message = (Message) messages.get(0); 
    messages.removeElementAt(0);

    return message; 
}

private void sendMessageToAllClients(Message message) {     
    for (int i=0; i < clients.size(); i++) { 
        Client client = (Client) clients.get(i); 
        client.sendMessage(message); 
    } 
}

public void run() { 
    try { 
        while (true) { 
            Message message = getNextMessageFromQueue(); 
            sendMessageToAllClients(message); 
        } 
    } catch (InterruptedException ie) {
        ie.printStackTrace();
    } 
}

Here the client class :

private Socket socket;

private ObjectOutputStream out;
private ObjectInputStream in;

public Client(Socket s) throws IOException {
    socket = s;

    out = new ObjectOutputStream(socket.getOutputStream());
    in = new ObjectInputStream(socket.getInputStream());
}

public Socket getSocket() {
    return socket;
}

public void sendMessage(Message message) { 
    try {
        out.writeObject(message);
        out.flush();
    } catch (IOException e) {
        e.printStackTrace();
    }           
}

Here is the main call of addMessage :

Message message = new Message();
message.setMessage("Welcome to " + client.getSocket().getLocalAddress() + ":" + client.getSocket().getPort());

dispatcher.addMessage(message);
asgoth
  • 35,552
  • 12
  • 89
  • 98
user1952589
  • 31
  • 3
  • 1
    Is there any other call to wait() somewhere else? You're not showing us the complete code. Might the call to `client.sendMessage()` block? Why is sendMessageToAllClients() synchronized? It doesn't use the vector. – JB Nizet Jan 06 '13 at 09:49
  • @user1952589 - Then please post that code also. – Mike Jan 06 '13 at 09:50
  • 1
    Why don't use [ArrayBlockingQueue](http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/ArrayBlockingQueue.html) take()/put()? (Vector is generic, Queue may be more appropriate) – ggrandes Jan 06 '13 at 09:55
  • I remember there's a question and answer which states that we should avoid `synchronized` methods and gave an example but *I just can't find it now* – Alvin Wong Jan 06 '13 at 09:55
  • can you println something to console immediately after `wait` line in order to see whether thread gets notified? – Mikita Belahlazau Jan 06 '13 at 10:05
  • 3
    It might be helpful if you came up with a simple complete example that demonstrated the problem and that we could experiment with. – NPE Jan 06 '13 at 10:14
  • 1
    @Alvin Wong, I think it is more generally about synchronizing around `this` (which is what method-level synchronization does) because clients may wish to synchronize against your object and thus unwillingly interfere with your own locking. – ignis Jan 06 '13 at 10:30
  • Can you try notifyAll() instead of notify(). – JoG Jan 06 '13 at 10:31
  • @user1952589 Did you make debug? – Mary Ryllo Jan 06 '13 at 11:53
  • @user1952589 Have you solved your problem? – Mary Ryllo Jan 06 '13 at 18:00

1 Answers1

1

I think you have a mistake in line in = new ObjectInputStream(socket.getInputStream()); Remove it, if it is not necessary or rebuild in other way. Read this Java sockets: Program stops at socket.getInputStream() w/o error?

To understand if your inputData is empty use - socket.getInputStream().available(), it returns size of input bytes.

Community
  • 1
  • 1
Mary Ryllo
  • 2,321
  • 7
  • 34
  • 53