Thread: Multi-Threaded Chat Server

I'm having problems with my ChatServer. I think I have done most of it correctly but it fails to print out to all active clients. It prints to each of them independently but just not as an overall broadcast. Here's what I've got:

1.) I have only a sample Client program to work with. The assignment was to build the Server side.
2.) I'm using a Queue to store messages sent by clients
3.) A class called ChatHandler is used to take care of the global queue
4.) A class, ActiveClient, deals with the incoming client requests on the server.

Can anyone tell me or advise me as to where the problem is? My assignment deadline is passed, but I am determined to solve this. Thanks in advance

import java.io.*;
import java.util.*;
import java.net.*;
 
//Handles the circulation of messages between the Server and its Clients
class ChatHandler
{
	private boolean isEmpty = true;	//is the queue empty?
	private String nextOut;	//next message to be printed
	private Queue<String> messages;	//the queue of messages
 
	public ChatHandler(Queue<String> q)
	{
		messages = q; //Take a copy of the global queue
		duplicate = q;
	}
 
	public synchronized String getNext() //Retrieve the next item off the queue
	{
		while(isEmpty)
		{
			try
			{
				wait(); //If empty, wait till something is put in
			}
			catch (InterruptedException e)
			{
				e.printStackTrace();
			}
		}
		nextOut = messages.remove(); //remove the longest waiting message
		if(messages.size() <= 0)
			isEmpty = true;	//If it was the last in the queue, set to empty
 
		notifyAll(); //Notify all waiting threads
 
		return nextOut;
	}
 
	public synchronized void writeTo(String msg) //Put a message on the queue
	{
		try
		{ //Prevent two or more threads accessing at once
			Thread.sleep((int)((Math.random()*100) + 1));
		}
		catch (InterruptedException e)
		{
			e.printStackTrace();
		}
		messages.add(msg); //Add to the queue
		isEmpty = false; //Queue cannot be empty now
		notifyAll(); //Notify all waiting threads
	}
}

//Thread to deal with new Client connections
class ActiveClient extends Thread
{
	private Socket inbound; //To create a bridge between machines
	private PrintWriter out; //Sends data to the socket
	private BufferedReader in; //Reads data from the socket
	private String userData; //Stores any message retrived
	private int id;
	private boolean isConnected; //Connection status indicator
	private ChatHandler myChat; //Handles messages circulation
 
	public ActiveClient(Socket s, ChatHandler ch, int i)
	{
		myChat = ch; inbound = s; id = i;
		try
		{ //Open the data in and data out streams
			in = new BufferedReader
				(new InputStreamReader(inbound.getInputStream()));
 
			out = new PrintWriter(inbound.getOutputStream());
			isConnected = true; //Connection is now active
		}
		catch (IOException e)
		{
			e.printStackTrace();
		}
	}
 
	int getIdNum()
	{
		return id;
	}
 
	public void printMessage() //Prints out chat messages to screen
	{
		if(out != null)
			//get the next message from the queue
			out.println("User "+ id + ": " + myChat.getNext());
			out.flush();
	}
 
	public boolean getStatus() //Check connection status
	{
		return isConnected;
	}
 
	public void run()
	{
		try
		{
			while(true)
			{ //While there is data to be read in...
				if((userData = in.readLine()) != null)
				{
					if(userData.equals("exit")) //If the user types exit, leave.
						break;
 
					myChat.writeTo(userData); //Else, put it on the queue
				}
 
				printMessage(); //Get and print the next item off the queue
			}
 
			inbound.close(); //When exit is called, close the socket
			System.out.println("User " + id + " has left.");
			isConnected = false; //Set the connection status to off
		}
		catch(IOException e)
		{
			e.printStackTrace();
		}
	}
}

class MyChat
{
	static void isDead(Set s) //Removes any dead clients
	{
		try
		{
			Iterator itr = s.iterator(); //Set iterator
			while(itr.hasNext())
			{
				ActiveClient temp = (ActiveClient) itr.next();
				if(!temp.getStatus()) //Check this clients status
				{
					s.remove(itr.next()); //If its dead, remove it.
					System.out.println("User " + temp.getIdNum() + ": has left.");
				}
			}
		}
		catch (Exception e)
		{
			e.printStackTrace();
		}
	}
 
	public static void main(String [] args)
	{
		Set<ActiveClient> live = new HashSet<ActiveClient>(); //Stores all Active Connections
		Queue<String> messageList = new LinkedList<String>(); //Global Message Queue
		ChatHandler global = new ChatHandler(messageList); //Handles the Queue
		ServerSocket server = null;
		Socket newConnection = null;
		int i = 1;
 
		try
		{
			server = new ServerSocket(7777);
 
			while(true)
			{
				newConnection = server.accept(); //Accept incoming connection
				ActiveClient next = new ActiveClient(newConnection, global, i++);
				live.add(next); //Add it to the set of Active Clients
				next.start(); //Start the Clients Thread
				isDead(live);
			}
		}
		catch (IOException e)
		{
		   System.err.println("Could not listen on port: 7777.");
    	   System.exit(-1);
		}
	}
}

Sorry, here is the Client Code I was using. Its an from an EchoClient / EchoServer program...

import java.io.*;
import java.net.*;
 
public class EchoClient {
 
    // Have a conversation with the echo server on vinson
    public static void main(String[] args) throws IOException {
 
	// We need a socket to talk to another machine
	Socket echoSocket = null;
 
	// We use a PrintWriter to write to the socket
	PrintWriter socketOut = null;
 
	// We use a BufferedReader to read from the socket
	BufferedReader socketIn = null;
 
        // We use a BufferedReader to read from the user
        BufferedReader userIn =
	    new BufferedReader(new InputStreamReader(System.in));
 
        // We store user input in a string
        String userInput;
 
        try {
 
	    // Create socket and connect to the echo server on vinson (port 7)
	    echoSocket = new Socket("localhost", 7777);
 
	    // Attach a printer to the socket's output stream
	    socketOut = new PrintWriter(echoSocket.getOutputStream(), true);
 
	    // Attach a reader to the socket's input stream
	    socketIn = new BufferedReader(
	        new InputStreamReader(echoSocket.getInputStream()));
 
        } catch (UnknownHostException e) {
 
	    // Simply exit if vinson is unreachable
	    System.err.println("Don't know about host: localhost");
            System.exit(1);
        }
 
        // Attach a reader to standard input
        userIn = new BufferedReader(new InputStreamReader(System.in));
 
        // While there is something to read from the user...
        while ((userInput = userIn.readLine()) != null) {
 
	    // Write it to the socket
	    socketOut.println(userInput);
 
	    // Print what we get back
	    System.out.println(socketIn.readLine());
        }
 
        // Close
        socketOut.close();
        socketIn.close();
        userIn.close();
        echoSocket.close();
    }
}

It prints to each of them independently but just not as an overall broadcast.

Can you explain this?

When I looked at your code, this is what I saw it does:

Multiple EchoClients can be started, and they allow users to input text into stdin (pardon the C++ lingo). When the user types something in, the client will take that line and write it out to stdout and also to port 7777.

MyChat will actively poll port 7777 for client connections. When it finds one, it will create a new ActiveClient thread and start it.

ActiveClient will monitor that connection for incoming data. If the data is "exit", it will break out of the loop, close that connection, and stop that ActiveClient thread. Otherwise, it will put the string onto the messages queue of the ChatHandler.

After that, the ActiveClient will call printMessages() which writes the oldest string from the messages queue to stdout (if any), or waits until there's something to write.

As far as I can tell, there's nothing wrong with your code (if what I've described is the expected behavior), though the thread sleeping in writeTo() is unnecessary since the method is synchronized. There's also a duplicate variable referenced in ChatHandler's constructor, but I can't find the declaration of that variable anywhere (I commented it out and everything worked as I described it).

edit:
I've removed the other post. The cross posting rule applies to asking questions on different sites (i.e. "JavaRanch.com" and "JavaProgrammingForums.com"), not in different categories (i.e. "What's wrong with my code?" and "Java Networking"). If you would like, I can move this thread over to that category for you.

Two reasons:
1. readLine is a blocking call. Therefore, the only time EchoClient can read something from the socket it's connected is in-between reads, when it's expecting to echo out what it just sent to MyChat. You'll want two threads for the client, one which will actively poll the port for any incoming data and reads it, and the other to poll stdin for user input and broadcast it back out the socket.

2. Your ActiveClient class will only write out to the connection it's connected to (due to the nature of the Socket class), not to all connections which happen to be on that port. If you want to write to all connections bound to that address/port, I would suggest using a MulticastSocket (see: Broadcasting to Multiple Recipients). One solution is to have all clients create two connections: one as you have right now, which allows clients to send data to MyChat, and a second MulticastSocket which will read data sent from MyChat. Alternatively, have your MyChat client write out to all ActiveConnections manually.

I'll see what I can do then. Thank you