Seems like I can't read from a scoket?

Hi

I'm currently trying to write a chat program, and a server program acting manager for the chat clients.

Evrything works just fine until I reach the chat part, the client can connect to the server, but it seems that the server never gets the message.

here is the code:

WARNING: At first i tryed to keep the coding in nice order and do evrything by the book, but as I've tryed to fix it MANY times, it ended up looking sort of like a mess. I've also done quite some things against the "rules" such as doing no recovery code to an exception.

import java.io.*;
import java.net.*;
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

public class SimpleChatClientA{
    JTextField outgoing;
    BufferedReader reader;
    PrintWriter writer;
    JTextArea incoming;
    Socket sock;
    String name;
    
    public void go(){
        name = null;
        JFrame frame = new JFrame("Ludicrously Sipmle Chat Client");
        JPanel mainPanel = new JPanel();
        outgoing = new JTextField(20);
        incoming = new JTextArea(15, 50);
        incoming.setLineWrap(true);
        incoming.setWrapStyleWord(true);
        incoming.setEditable(false);
        JScrollPane qScroller = new JScrollPane(incoming);
        qScroller.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
        qScroller.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER);
        JButton sendButton = new JButton("send");
        sendButton.addActionListener(new SendButtonListener());
        mainPanel.add(qScroller);
        mainPanel.add(outgoing);
        mainPanel.add(sendButton);
        frame.getContentPane().add(BorderLayout.CENTER, mainPanel);
        
        incoming.append("You must write your name before network with the server can be established" + "\n"); 
        
        frame.setSize(600, 500);
        frame.setVisible(true);
    }
    
    private void setUpNetworking(){
        if(name != null){
            try{
                sock = new Socket("127.0.0.1", 5323);
                InputStreamReader streamReader = new InputStreamReader(sock.getInputStream());
                reader = new BufferedReader(streamReader);
                writer = new PrintWriter(sock.getOutputStream());
                incoming.append("Netork established" + "\n");
            }catch(IOException ex){
                ex.printStackTrace();
            }
        }
    }
    
    public class SendButtonListener implements ActionListener{
        public void actionPerformed(ActionEvent ev){
            if(name != null && outgoing.getText() != null){
                    try{
                        writer.println(outgoing.getText());
                        writer.flush();
                    } catch(Exception ex){
                        ex.printStackTrace();
                    }
                    outgoing.setText("");
                    outgoing.requestFocus();
                }else if(name == null){
                    name = outgoing.getText();
                    incoming.append("your name is now " + name + "\n");
                    outgoing.setText("");
                    outgoing.requestFocus();
                    setUpNetworking();
                    Thread readerThread = new Thread(new IncomingReader());
                    readerThread.start();
                }
        }
    }
    
    public class IncomingReader implements Runnable{
        public void run(){
            String message;
            try{
                while((message = reader.readLine()) != null){
                    incoming.append("read " + message);
                    incoming.append(message + "\n");
                }
            }catch(Exception ex){ex.printStackTrace();}
        }
    }
    
    public static void main(String[] args){
        new SimpleChatClientA().go();
    }
}

and the server:

import java.io.*;
import java.net.*;
import javax.swing.*;
import java.awt.*;
import java.util.*;

class ServerSide {

public ArrayList<BufferedReader> clientList;
    BufferedReader reader;
    PrintWriter writer;
    ServerSocket serverSock;
    Socket sock;
    JFrame frame;
    JTextArea incoming;
    ClientHandeler handeler;
    
    public void setup(){
        frame = new JFrame("Chat server");
        incoming = new JTextArea(15, 50);
        incoming.setLineWrap(true);
        incoming.setWrapStyleWord(true);
        incoming.setEditable(false);
        JScrollPane qScroller = new JScrollPane(incoming);
        qScroller.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
        qScroller.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER);
        handeler = new ClientHandeler();
        frame.getContentPane().add(BorderLayout.CENTER, qScroller);
        frame.setBounds(20, 20, 500, 500);
        frame.setVisible(true);
        Thread thread = new Thread(handeler);
        thread.start();
        setupServer();
    }
        
    
    public void setupServer(){
        clientList = new ArrayList<BufferedReader>();
        try{
            serverSock = new ServerSocket(5323);
            incoming.append("Server online" + "\n");
            while(true){
                
                sock = serverSock.accept();
                
                if(sock != null){
                    Client c = new Client(sock);
                    clientList.add(new BufferedReader(new InputStreamReader(sock.getInputStream())));
                    incoming.append("A client has connected to the server from: " + sock.getLocalAddress() + "\n");
                    sock = null;
                }
            }
        }catch(IOException ex){
            //if(serverSock == null){
                incoming.append("Server is offline" + "\n");
            //}
        }
        
    }
    
    public static void main(String[] args){
        new ServerSide().setup();
    }

public class ClientHandeler implements Runnable{

public void run(){
        
        incoming.append("Client handeler is running!" + "\n");
        
        while(true){
            if(clientList.isEmpty() != true){
                try{
                for(BufferedReader c : clientList){
                        String message;
                        
                        if((message = reader.readLine()) != ""){
                            incoming.append("A client wrote: " + message + "\n");
                            //tellAll(message);
                }
            }
        }catch(Exception ex){}
        }
    }
}}

//public void tellAll(String messsage){
        //if(clientList.isEmpty() == false){
                //for(Client c : clientList){
                   // try{
                        //Socket sock = c.getSocket();
                        //PrintWriter writer = new PrintWriter(new OutputStreamWriter(sock.getOutputStream()));
                       
                        //String message;
                        
                        //if((message = reader.readLine()) != null){
                           // tellAll(message);
                       // }
                        
                   // }catch(Exception ex){}
                //}
           // }
    //}

public class Client{

Socket sock;
    
    Client(Socket socket){
        sock = socket;
    }
    
   public Socket getSocket(){
       return sock;
    }
}
}

I hope it was not too messy.

Anyway, does anyone know what is wrong here?

Thanks in advance!

the main problem is in your setupServer() method.

You create a ServerSocket and start listening on it.

Then, when a client comes in, the server accepts the client,
writes a message and sets the client socket (sock) to null.

You already have a ClientHandler but you are not using it.

Your server should run in loop after setting up the listen
socket (ServerSocket).
For each incoming client it should then create a new
ClientHandler and pass the client socket (Socket)
to that client handler.

Then it will work.

Another problem is:
your server and client are Java programs running in
a JVM each. When closing the window, the JVM does not
get terminated (System.exit()).
You can fix that by adding

to your code.

Hope that helps
Matt

Thanks you very much!

It helped quite a lot

And thanks for the thing about "killing" the JVM, before I had to manualy shut it down each time I tested it .

Okay so I re-wrote a lot of the code and it ended up like this:

import java.io.*;
import java.net.*;
import javax.swing.*;
import java.awt.*;
import java.util.*;

class ServerSide {

ArrayList<ClientHandeler> handelerList;
    ArrayList<Thread> threadList;
    PrintWriter writer;
    ServerSocket serverSock;
    Socket sock;
    JFrame frame;
    JTextArea incoming;
    ClientHandeler handeler;
    
    public void setup(){
        frame = new JFrame("Chat server");
        frame.setDefaultCloseOperation(frame.EXIT_ON_CLOSE);
        incoming = new JTextArea(15, 50);
        incoming.setLineWrap(true);
        incoming.setWrapStyleWord(true);
        incoming.setEditable(false);
        JScrollPane qScroller = new JScrollPane(incoming);
        qScroller.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
        qScroller.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER);
        frame.getContentPane().add(BorderLayout.CENTER, qScroller);
        frame.setBounds(20, 20, 500, 500);
        frame.setVisible(true);
        setupServer();
    }
        
    
    public void setupServer(){
        threadList = new ArrayList<Thread>();
        handelerList = new ArrayList<ClientHandeler>();
        try{
            serverSock = new ServerSocket(5323);
            incoming.append("Server online" + "\n");
            while(true){
                
                sock = serverSock.accept();
                
                if(sock != null){
                    ClientHandeler handeler = new ClientHandeler(sock);
                    Thread thread = new Thread(handeler);
                    thread.start();
                    handelerList.add(handeler);
                    threadList.add(thread);
                    incoming.append("A client has connected to the server from: " + sock.getLocalAddress() + "\n");
                    sock = null; 
                    handeler = null;
                    thread = null;
                }
            }
        }catch(IOException ex){
                incoming.append("Server is offline" + "\n");
        }
        
    }
    
    public void tallAll(String message){
        
    }
    
    public static void main(String[] args){
        new ServerSide().setup();
    }

public class ClientHandeler implements Runnable{

Socket cliSock;
        BufferedReader reader;
        PrintWriter writer;

public ClientHandeler(Socket socket){
            cliSock = socket;
        }
        
        public void writeToClient(String message){
            try{
                writer.println(message);
                writer.flush();
            }catch(Exception ex){
                ex.printStackTrace();
            }
        }

public void run(){
        
            incoming.append("Client handeler is running!" + "\n");
            try{
                reader = new BufferedReader(new InputStreamReader(cliSock.getInputStream()));
                writer = new PrintWriter(new OutputStreamWriter(cliSock.getOutputStream()));
                
                String message;
                boolean WriteTo = false;
            
                while(true){
                    if((message = reader.readLine()) != null){
                        incoming.append(message + "\n");
                        for(ClientHandeler h : handelerList){
                            h.writeToClient(message);
                        }
                        message = null;
                    }
                
                }
            }catch(Exception ex){
                ex.printStackTrace();
            }
        }
    }
}

import java.io.*;
import java.net.*;
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

public class SimpleChatClientA{
    JTextField outgoing;
    BufferedReader reader;
    PrintWriter writer;
    JTextArea incoming;
    Socket sock;
    String name;
    
    public void go(){
        name = null;
        JFrame frame = new JFrame("Ludicrously Sipmle Chat Client");
        frame.setDefaultCloseOperation(frame.EXIT_ON_CLOSE);
        JPanel mainPanel = new JPanel();
        outgoing = new JTextField(20);
        incoming = new JTextArea(15, 50);
        incoming.setLineWrap(true);
        incoming.setWrapStyleWord(true);
        incoming.setEditable(false);
        JScrollPane qScroller = new JScrollPane(incoming);
        qScroller.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
        qScroller.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER);
        JButton sendButton = new JButton("send");
        sendButton.addActionListener(new SendButtonListener());
        mainPanel.add(qScroller);
        mainPanel.add(outgoing);
        mainPanel.add(sendButton);
        frame.getContentPane().add(BorderLayout.CENTER, mainPanel);
        
        incoming.append("You must write your name before network with the server can be established" + "\n"); 
        
        frame.setSize(600, 500);
        frame.setVisible(true);
    }
    
    private void setUpNetworking(){
        if(name != null){
            try{
                sock = new Socket("myIpAdress", 5323);
                InputStreamReader streamReader = new InputStreamReader(sock.getInputStream());
                reader = new BufferedReader(streamReader);
                writer = new PrintWriter(sock.getOutputStream());
                incoming.append("Netork established" + "\n");
            }catch(IOException ex){
                ex.printStackTrace();
            }
        }
    }
    
    public class SendButtonListener implements ActionListener{
        public void actionPerformed(ActionEvent ev){
            if(name != null && outgoing.getText() != null){
                    try{
                        writer.println(name + ": " +  outgoing.getText());
                        writer.flush();
                    } catch(Exception ex){
                        ex.printStackTrace();
                    }
                    outgoing.setText("");
                    outgoing.requestFocus();
                }else if(name == null){
                    name = outgoing.getText();
                    incoming.append("your name is now " + name + "\n");
                    outgoing.setText("");
                    outgoing.requestFocus();
                    setUpNetworking();
                    Thread readerThread = new Thread(new IncomingReader());
                    readerThread.start();
                }
        }
    }
    
    public class IncomingReader implements Runnable{
        public void run(){
            String message;
            try{
                while((message = reader.readLine()) != null){
                    //incoming.append("read " + message);
                    incoming.append(message + "\n");
                }
            }catch(Exception ex){ex.printStackTrace();}
        }
    }
    
    public static void main(String[] args){
        new SimpleChatClientA().go();
    }
}

(The ip adress is replaced by my ip adress, which i found by googeling "my ipadress" and a coulpe of homepages told me the same adress, so I just took that)

It works fine when i try to connect from my laptop, or with local adress on my own machine, but a sent the client to two friends, but none of them could connect.

Is there something special to know when people are connecting from another network? (thier firewall was turned off during the try)

let's deal with the code first

Nice job Hans, you got it working!

your setupServer() method really should only set up the server

you could move the threadList, handlerList and serverSocket instantiation to another method and rename it (e.g. serverLoop())

try to separate the GUI (Swing) part from everything else, makes it easier when it comes to dealing with telAll() etc

And:
what if a thread looses connection? will it be properly terminated and removed from the list?

You could introduce a thread group and a vulture thread for that group that goes for all
the cadavers you leave behind

I find David Flanagan's example quite useful.

// This example is from the book _Java in a Nutshell_ by David Flanagan.
// Written by David Flanagan.  Copyright (c) 1996 O'Reilly & Associates.
// You may study, use, modify, and distribute this example for any purpose.
// This example is provided WITHOUT WARRANTY either expressed or implied.

import java.net.*;
import java.io.*;
import java.awt.*;
import java.util.*;

public class Server extends Thread {
    public final static int DEFAULT_PORT = 6789;
    protected int port;
    protected ServerSocket listen_socket;
    protected ThreadGroup threadgroup;
    protected List connection_list;
    protected Vector connections;
    protected Vulture vulture;
    
    // Exit with an error message, when an exception occurs.
    public static void fail(Exception e, String msg) {
        System.err.println(msg + ": " +  e);
        System.exit(1);
    }
    
    // Create a ServerSocket to listen for connections on;  start the thread.
    public Server(int port) {
        // Create our server thread with a name.
        super("Server");
        if (port == 0) port = DEFAULT_PORT;
        this.port = port;
        try { listen_socket = new ServerSocket(port); }
        catch (IOException e) { fail(e, "Exception creating server socket"); }
        // Create a threadgroup for our connections
        threadgroup = new ThreadGroup("Server Connections");

// Create a window to display our connections in
        Frame f = new Frame("Server Status");
        connection_list = new List();
        f.add("Center", connection_list);
        f.resize(400, 200);
        f.show();

// Initialize a vector to store our connections in
        connections = new Vector();

// Create a Vulture thread to wait for other threads to die.
        // It starts itself automatically.
        vulture = new Vulture(this);

// Start the server listening for connections
        this.start();
    }
    
    // The body of the server thread.  Loop forever, listening for and
    // accepting connections from clients.  For each connection, 
    // create a Connection object to handle communication through the
    // new Socket.  When we create a new connection, add it to the
    // Vector of connections, and display it in the List.  Note that we
    // use synchronized to lock the Vector of connections.  The Vulture
    // class does the same, so the vulture won't be removing dead 
    // connections while we're adding fresh ones.
    public void run() {
        try {
            while(true) {
                Socket client_socket = listen_socket.accept();
                Connection c = new Connection(client_socket, threadgroup,
                                  3, vulture);
                // prevent simultaneous access.
                synchronized (connections) {
                    connections.addElement(c);
                    connection_list.addItem(c.toString());
                }
            }
        }
        catch (IOException e) {
            fail(e, "Exception while listening for connections");
        }
    }
    
    // Start the server up, listening on an optionally specified port
    public static void main(String[] args) {
        int port = 0;
        if (args.length == 1) {
            try { port = Integer.parseInt(args[0]); }
            catch (NumberFormatException e) { port = 0; }
        }
        new Server(port);
    }
}

// This class is the thread that handles all communication with a client
// It also notifies the Vulture when the connection is dropped.
class Connection extends Thread {
    static int connection_number = 0;
    protected Socket client;
    protected Vulture vulture;
    protected DataInputStream in;
    protected PrintStream out;

// Initialize the streams and start the thread
    public Connection(Socket client_socket, ThreadGroup threadgroup,
              int priority, Vulture vulture) 
    {
        // Give the thread a group, a name, and a priority.
        super(threadgroup, "Connection-" + connection_number++);
        this.setPriority(priority);
        // Save our other arguments away
        client = client_socket;
        this.vulture = vulture;
        // Create the streams
        try { 
            in = new DataInputStream(client.getInputStream());
            out = new PrintStream(client.getOutputStream());
        }
        catch (IOException e) {
            try { client.close(); } catch (IOException e2) { ; }
            System.err.println("Exception while getting socket streams: " + e);
            return;
        }
        // And start the thread up
        this.start();
    }
    
    // Provide the service.
    // Read a line, reverse it, send it back.  
    public void run() {
        String line;
        StringBuffer revline;
        int len;

// Send a welcome message to the client
        out.println("Line Reversal Server version 1.0");
        out.println("A service of O'Reilly & Associates");

try {
            for(;;) {
                // read in a line
                line = in.readLine();
                if (line == null) break;
                // reverse it
                len = line.length();
                revline = new StringBuffer(len);
                for(int i = len-1; i >= 0; i--) 
                    revline.insert(len-1-i, line.charAt(i));
                // and write out the reversed line
                out.println(revline);
            }
        }
        catch (IOException e) { ; }
        // When we're done, for whatever reason, be sure to close
        // the socket, and to notify the Vulture object.  Note that
        // we have to use synchronized first to lock the vulture
        // object before we can call notify() for it.
        finally {
            try { client.close(); } catch (IOException e2) { ; }
            synchronized (vulture) { vulture.notify(); }
        }
    }

// This method returns the string representation of the Connection.
    // This is the string that will appear in the GUI List.
    public String toString() {
        return this.getName() + " connected to: " 
            + client.getInetAddress().getHostName()
            + ":" + client.getPort();
    }
}

// This class waits to be notified that a thread is dying (exiting)
// and then cleans up the list of threads and the graphical list.
class Vulture extends Thread {
    protected Server server;

protected Vulture(Server s) {
        super(s.threadgroup, "Connection Vulture");
        server = s;
        this.start();
    }

// This is the method that waits for notification of exiting threads
    // and cleans up the lists.  It is a synchronized method, so it
    // acquires a lock on the `this' object before running.  This is 
    // necessary so that it can call wait() on this.  Even if the 
    // the Connection objects never call notify(), this method wakes up
    // every five seconds and checks all the connections, just in case.
    // Note also that all access to the Vector of connections and to
    // the GUI List component are within a synchronized block as well.
    // This prevents the Server class from adding a new conenction while
    // we're removing an old one.
    public synchronized void run() {
        for(;;) {
            try { this.wait(5000); } catch (InterruptedException e) { ; }
            // prevent simultaneous access
            synchronized(server.connections) {
                // loop through the connections
                for(int i = 0; i < server.connections.size(); i++) {
                    Connection c;
                    c = (Connection)server.connections.elementAt(i);
                    // if the connection thread isn't alive anymore, 
                    // remove it from the Vector and List.
                    if (!c.isAlive()) {
                        server.connections.removeElementAt(i);
                        server.connection_list.delItem(i);
                        i--;
                    }
                }
            }
        }
    }
}

The Connection Thread object is the equivalent to your ClientHandler.

Matt

and now about the network problem

let's assume you are connected to the Net via DSL.
Event hough this connection is durable your provider
probably issues you a new IP address every now and then.

Unless you are using a static IP address from your ISP or
dynamic DNS the IP address your friends use to connect
will change.

So this is how we can get to your front door.

Then there is the firewall.

You say it works from your laptop. So I assume you are connecting
from your laptop to a 192.168.x.x or 10.x.x.x address.

These are private addresses and cannot be routed to the Net.

You will have to enable port forwarding on the firewall for port 5323 and
have it pointing to the IP address you can connect from your laptop.
This should be the same address used by your server ("myIpAddress").

Hope that helps
Matt

So if I want people from the outside world to connect to my server, I have to mess with port forwarding?

that is one of the many options

let's assume you have a switch / hub with a built-in DSL modem

port forwarding is the recommended way

you also could configure the chat machine to be in the Demilitarized Zone (DMZ)

In the latter case you have to make sure your firewall is tight.

I have two SHH port-forwarders configured and one for HTTP.
Every night I have an average of 1,800 break in attempts
between them.

Maybe you should implement a bit of protocol on your chat server
to detect valid connect requests.

Password authentication would be a plus before you allow any
client to send and receive chat messages.

Hope that helps
Matt

I think I have just to try make the server better, and acept that I can't connent with people from the outside world. I don't really know much about internet related stuff like DMZ, and beside from that, I can't even connect to my router as I don't have the password for it.

But I'll try to improve the server.

By the way, i find this forum really helpfull and friendly, thansk for the nice responses