When my socket client stops, "Socket closed Exception" is thrown

[Joseph Hwang]

First, Below codes are simple Java socket programming,

==== Server Codes =====

public class JavaChatServer {
	
	private static final int PORT = 10001;
	
	public static void main(String[] args) {
		// TODO Auto-generated method stub
		ServerSocket server = null;
		HashMap<String, PrintWriter> hm = null;
		
		try {
			server = new ServerSocket(PORT);
			hm = new HashMap<String, PrintWriter>();
			
			while(true) {
				System.out.println("Waiting for Connection...");
				Socket socket = server.accept();
				ChatThread chatThread = new ChatThread(socket, hm);
				chatThread.start();
			}
		}catch (Exception e) {
			e.printStackTrace();
		} finally {
			try {
				if (!server.isClosed()) {
					server.close();
					System.out.println("The Chatting Server stops!");	
				}
				
			} catch (Exception e1) {
				e1.printStackTrace();
			}
		}
	}

}

class ChatThread extends Thread {
	private Socket socket;
	private String id;
	private BufferedReader br;
	private PrintWriter pw;
	private HashMap<String, PrintWriter> hm;

	public ChatThread(Socket socket, HashMap<String, PrintWriter> hm) {
		this.socket = socket;
		this.hm = hm;
	}
	
	@Override
	public void run() {
		try {
			pw = new PrintWriter(new OutputStreamWriter(socket.getOutputStream()));
			br = new BufferedReader(new InputStreamReader(socket.getInputStream()));
			id = br.readLine();
			
			broadcast(id + " is connected.");
			System.out.println("The connected User ID : " + id);
			synchronized (hm) {
				hm.put(this.id, pw);
			}
			
			String line = null;
			
			while(true) {
				line = br.readLine();
				if(line.equals("/quit")) {
					break;
				}
			}
		} catch (Exception e) {
			e.printStackTrace();
		} finally {
			synchronized (hm) {
				hm.remove(id);
			}
			broadcast(id + " is disconnected.");
			
			try {
				if(socket != null) {
					socket.close();
				}
			} catch (Exception e) {
				e.printStackTrace();
			}
		}
	}
	
	public void broadcast(String message) {
		synchronized (hm) {
			Collection<PrintWriter> collection = hm.values();
			Iterator<PrintWriter> iterator = collection.iterator();
			
			while(iterator.hasNext()) {
				PrintWriter pw = iterator.next();
				pw.println(message);
				pw.flush();
			}
		}
	}
}

I think the above server application codes work well, It throws no exceptions. But the problem is that the below client socket codes throws "Socket closed Exception" when the client stops.

==== Client Codes ======

public class JavaChatClient {

	public static void main(String[] args) {
		// TODO Auto-generated method stub
		if(args.length != 2) {
			System.out.println("Usage : java JavaChatClient id ip_address");
			System.exit(1);
		}
		
		Socket socket = null;
		BufferedReader br = null;
		PrintWriter pw = null;
		
		try {
			socket = new Socket(args[1], 10001);
			pw = new PrintWriter(new OutputStreamWriter(socket.getOutputStream()));
			br = new BufferedReader(new InputStreamReader(socket.getInputStream()));
			
			pw.println(args[0]);
			pw.flush();
			
			InputThread it = new InputThread(socket, br);
			OutputThread ot = new OutputThread(socket, pw);
			
			it.start();
			ot.start();
		} catch(Exception e) {
			e.printStackTrace();
		} finally {
			try {
				if(socket.isClosed()) {
					socket.close();
				}
			} catch (Exception e) {
				e.printStackTrace();
			}
		}
	}
}

class InputThread extends Thread {
	
	private Socket socket = null;
	private BufferedReader br = null;
	
	public InputThread(Socket socket, BufferedReader br) {
		this.socket = socket;
		this.br = br;
	}
	
	@Override
	public void run() {
		try {
			String line = null;
			
			while(true) {
				line = br.readLine(); [b][u]// This line throws exception[/u][/b]
				System.out.println(line);	
			}
		} catch (Exception e) {
			e.printStackTrace();
		} finally {
			try {
				if(br != null) {
					br.close();
				}
				if(!socket.isClosed()) {
					socket.close();
				}
			} catch (Exception e) {
				e.printStackTrace();
			}
		}
	}
}

class OutputThread extends Thread {
	
	private Socket socket = null;
	private PrintWriter pw = null;
	private BufferedReader keyboard = null;
	
	public OutputThread(Socket socket, PrintWriter pw) {
		this.socket = socket;
		this.pw = pw;
		
		keyboard = new BufferedReader(new InputStreamReader(System.in));
	}
	
	@Override
	public void run() {
		try {
			String line = null;
			while(true) {
				line = keyboard.readLine();
				
				pw.println(line);
				pw.flush();
				
				if(line.equals("/quit")) {
					break;
				}
			}
		} catch (Exception e) {
			e.printStackTrace();
		} finally {
			try {
				if(pw != null) {
					pw.close();
				}
				if(keyboard != null) {
					keyboard.close();
				}
				if(!socket.isClosed()) {
					socket.close();
				}
			} catch (Exception e) {
				e.printStackTrace();
			}
		}
	}
}

The exception is
java.net.SocketException: Socket closed
       at java.net.SocketInputStream.socketRead0(Native Method)
       at java.net.SocketInputStream.socketRead(Unknown Source)
       at java.net.SocketInputStream.read(Unknown Source)
       at java.net.SocketInputStream.read(Unknown Source)
       at sun.nio.cs.StreamDecoder.readBytes(Unknown Source)
       at sun.nio.cs.StreamDecoder.implRead(Unknown Source)
       at sun.nio.cs.StreamDecoder.read(Unknown Source)
       at java.io.InputStreamReader.read(Unknown Source)
       at java.io.BufferedReader.fill(Unknown Source)
       at java.io.BufferedReader.readLine(Unknown Source)
       at java.io.BufferedReader.readLine(Unknown Source)
       at com.aaa.client.InputThread.run(JavaChatClient.java:66)

It seems when the "br.readLine()" line is executed, the the socket of BufferedReader objects br is already closed. So it throws Socket closed Exception.
I'd like to know how to close the BufferedReader br object automatically when the client socket connection is disconnected.
I am totally stuck here. Any advice will be appreciated. Thanks

[Stephan van Hulst]

Yes, you are closing the socket prematurely on line 32.

Remember that your sockets are read from in separate threads, and the original thread continues with the finally clause directly after it's started the threads.

You need to wait until both the input and output threads are done before you close the socket. All of this is much easier if you use a GUI rather than System.in, because with a GUI your output can be event-driven, rather than having to use a separate thread to read from System.in. Remember though, if you want to continue with the System.in approach, do not close it afterwards, as you're doing now. System.in should never be closed.

[Joseph Hwang]

Thank you for your reply. I solved this issue by simple modification like below,

//it.start()
ot.start(); 
it.start();

I put the input thread start line to the next of output thread line. And It works. ;)

[Stephan van Hulst]

That's not a reliable fix. In theory the two threads still start at around the same time, so any difference in behavior is just because of race conditions.