Multi-threading and Java Swing Problem

Question

Hi I have a GUI application that is working fine. I created a socket server. When I create a new object of the Server class in program the GUI application stops responding.

This is my server class. If I do

Server s = new Server();

in my main application it stops working. How should I add it? Making a new thread? I tried

Thread t = new Thread(new Server());
t.start();

but the problem persisted. Please, I'll appreciate your help.

package proj4;

import java.net.*; 
import java.io.*; 

public class Server implements Runnable { 
    ServerSocket       serverSocket = null;
    Socket             clientSocket = null;
    ObjectOutputStream out          = null;
    ObjectInputStream  in           = null;
    int                port;
    static int         defaultPort  = 30000;
    boolean            isConnected  = false;
    Thread             thread;
    DataPacket         packet       = null;

    public Server(int _port) {
        try {
            serverSocket = new ServerSocket(_port);
            serverSocket.setSoTimeout(1000*120);  //2 minutes time out     
            isConnected = true;
            System.out.println("server started successfully");
            thread = new Thread(this);
            thread.setDaemon(true);
            //thread.run();
        } catch (IOException e) {
            System.err.print("Could not listen on port: " + port);
            System.exit(1);
        }
        try {
            System.out.println("Waiting for Client");
            clientSocket = serverSocket.accept();
            System.out.println("Client Connected");
            thread.run();
        } catch (IOException e) {
            System.err.println("Accept failed.");
            System.exit(1);
        }
        try {
            out = new ObjectOutputStream(clientSocket.getOutputStream());
            System.out.println("output stream created successfully");
        } catch (IOException e) {
            e.printStackTrace();
        }
        try {
            in = new ObjectInputStream(clientSocket.getInputStream());
            System.out.println("input stream created successfully");
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public Server() {
        this(defaultPort); //server listens to port 30000 as default
    }

    public void run() {
        System.out.println("Thread running, listening for clients");//debugging purposes
        while (isConnected) {
            try {
                packet = this.getData();
                Thread.sleep(0);
            } catch(InterruptedException e) {
                e.printStackTrace();
            }
        }
    } 

    public DataPacket getData() {
        try {
            packet = (DataPacket)in.readObject();
        } catch (Exception ex)  {
            System.out.println(ex.getMessage());
        }
        return packet;
    }

    public void sendData(DataPacket dp) {
        try {
            out.writeObject(dp);
        } catch (IOException e) {
            e.printStackTrace();
        } 
        try {
            out.flush();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public void closeConnection() throws IOException {
        out.close(); 
        in.close(); 
        clientSocket.close(); 
        serverSocket.close(); 
    }
}

Answer 1

Your Server constructor blocks, potentially indefinitely, in accept().

Two things about Swing programs:

1. Never do any long task in the Swing event thread, and,
2. Never do manipulate any Swing object outside the Swing event thread unless the method being used is explicitly documented as thread-safe.

This means that if the server is being started from the Swing event thread -- that is, if it is being started in response to a button click or the like -- then yes you must spawn another thread for your Server object. Otherwise you guarantee that the Swing event thread will be blocked until your thread exits.

You say that your application still stops responding even when you spawn another thread for your server? Ensure that you're calling Thread.start() and not run(), or you will accidentally still block yourself by running the "new Thread" actually in your own Thread.

NOTES:

1. I see that you do a Thread.sleep(0); in your run() loop. This is not guaranteed to do anything whatsoever. If you have a single CPU machine, this may be fairly be implemented as a no-op, allowing the same thread to keep running.
2. You really want isConnected to be volatile -- otherwise there is no guarantee that changes to this variable will be seen by any thread other than the one where it is changed.
3. You don't set isConnected to false, anywhere, so your run() will run until the JVM is stopped or until that Thread takes a RuntimeException.
4. It is discouraged to start Threads in constructors. (See Java Concurrency In Practice.)
5. You don't want to accept on your ServerSocket until you are in your Thread's run() method! Otherwise your constructor will block waiting for a connection and will not return control to the event thread!
6. You have the following code in your constructor:

your code is:

thread = new Thread(this);
thread.setDaemon(true);
//thread.run();

When you had thread.run() not commented out, you were not starting a new Thread! To do that, you need to do thread.start(). Instead, you were running this new Thread (which will never stop, for reason #3 above) in the same thread that invoked the constructor. The way your code is written right now, all IOExceptions are logged, but otherwise swallowed. You probably want to set isConnected to false on any IOException, as well as in closeConnection().

Answer 2

Your server class looks plausible (I haven't actually compiled it, but it looks plausible.) If your GUI is becoming unresponsive, that pretty much means the GUI thread isn't getting control. To see why, we really need a look at the code that creates this -- where it is and how it works.

Your basic idea is right, although you can shorten it to

(new Thread(new Server()).start();

That's certainly not the issue, though.

Here's a thought: at the point you create and start the server, directly after the call, add a print or logging statement, ie,

(new Thread(new Server()).start();
System.err.println("Got here!");

and see if you see the "Got here!" message. If not, then you're blocking the GUI thread.

Answer 3

You can use SwingWorker which is available in Java 6. If you need to use it in a previous version of Java you can always use:

https://swingworker.dev.java.net/

SwingWorker is probably the cleaner approach. You can find more info about it here:

http://java.sun.com/products/jfc/tsc/articles/threads/threads2.html

You can then write a new ServerSwingWorker inner class to wrap that functionality or use an anonymous inner class.

Answer 4

The problem is that your constructor for Server is blocking. The cosntructor should not make any blocking calls (and in fact it should do as little as possible). The blocking calls should be made by run() or by something called by run().

Also note that you create a new Thread() is the constructor that doesn't seem to have any purpose.

Answer 5

little detail, doing

    new Thread(new Server())

won't help since the "new Server()" is still executed in the actual Thread.

It should be something like:

    Thread t = new Thread(new Runnable() {
        @Override
        public void run() {
            new Server();
        }
    });
    t.start();