1

This is the first java socket/multithreaded application that I write, therefore I would like to apologize for the atrocious code that you are about to witness.

Anyway, most of you will probably regard this code as being basic, a standard server that allows connection from more clients at a time. Also, the server has an interface with just a StopServer button, which closes the server, meanwhile the Client doesn't do anything else than just connect to the server and then disconnect afterwards.

Now, if I simply run the server class, it's ok, nothing 'bad' happens and when I close it, it closes fine, however:

1: If I run the server class, and then I run the client class once, let the client disconnect, and then try to close the server, I get the error:

java.net.SocketException: socket closed

2: Each client will add about ~30-35% of CPU utilization in just a brief run, and that utilization will remain at the "Java(TM) Platform SE Binary" process, for as long as the server continues to run. If I let a client be connected to the server for, let's say 30 seconds, the CPU utilization will reach 100%.

Also, I did a little research and I know that the "socket closed exception" means that you closed the socket, and then continued to try to use it, and also there's probably something wrong with how the server handles the disconnected clients.

Here's the code:

Server

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



public class Server extends JFrame
    { private Connection con;
    private static int port = 44444;
    private boolean serverKeepGoing;
    private static int uniqueId;
    private ArrayList<ClientThread> al;
    private ServerSocket serverSocket;
    public Scanner keyboard = new Scanner(System.in);

    public static void main(String[] args) throws IOException 
        { Server server = new Server(port);
          server.start();

        }


    public void ServerClose()
        { 
            serverKeepGoing = false;
            try 
            { 
            for(int i = 0; i < al.size(); ++i) 
                { ClientThread tc = al.get(i);
                try 
                    { 
                    tc.in.close();
                    tc.out.close();
                    tc.socket.close(); }
                catch(IOException e) { e.printStackTrace(); } 

                serverSocket.close();}

            }catch(Exception e) { e.printStackTrace(); }
        }


    public Server (int port)
        { 

        serverInterface();
        al = new ArrayList<ClientThread>();
        }




     public void start() 
            { serverKeepGoing = true;

            try 
            { serverSocket = new ServerSocket(port);
              System.out.println("Server is running!");

                while(serverKeepGoing) 
                { Socket socket = serverSocket.accept(); // accept connection. LINE 65
                    // ^ALSO :java.net.SocketException: socket closed
                    // if I was asked to stop



                if(!serverKeepGoing)
                    { ServerClose(); break;}

                    ClientThread t = new ClientThread(socket);  // make a thread of it
                    al.add(t);                                  // save it in the ArrayList
                    t.start();


                }



                ServerClose();  // means the server has got to be closed

            }catch (IOException e) { e.printStackTrace(); System.out.println("Error in method start"); }


        }   


    public synchronized void remove(int id) {
        // scan the array list until we found the Id
        for(int i = 0; i < al.size(); ++i) {
            ClientThread ct = al.get(i);
            // found it
            if(ct.id == id) {
                al.remove(i);
                return;
            }
        }
    }



    class ClientThread extends Thread 
        { // the socket where to listen/talk
        Socket socket;
        BufferedReader in;
        PrintWriter out;
        boolean clientKeepGoing;
        // my unique id (easier for deconnection)
        int id;


        public ClientThread(Socket socket) 
            { id = ++uniqueId;
            this.socket = socket;

            try
            {

                 in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
                 out = new PrintWriter(socket.getOutputStream(), true);


            }
            catch (IOException e) { return; }


        }


        public void run() 
            { 
            boolean clientKeepGoing = true;
            while(clientKeepGoing) 
                { try
                    {   





                    }catch(Exception e){ e.printStackTrace(); }


                }
            // remove myself from the arrayList containing the list of the
            // connected Clients
            remove(id);
            close();
            }


        // try to close everything
        private void close() 
            { clientKeepGoing = false;
            try {
                if(out != null) out.close();
            }
            catch(Exception e) {}
            try {
                if(in != null) in.close();
            }
            catch(Exception e) {};
            try {
                if(socket != null) socket.close();
            }
            catch (Exception e) {}

            }


    }

    public void serverInterface(){
        JFrame frame = new JFrame("Server");

        frame.setLayout(null);

        int windowWidth = 300;
        int windowHeight = 400;

        frame.setBounds(250, 150, windowWidth, windowHeight);

        JButton stopServer = new JButton("Stop server");

        stopServer.setFocusable(false);

        stopServer.setBounds(60, 275, 175, 20);

        frame.add(stopServer);

        stopServer.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e)
            {
                ServerClose();
                System.exit(1);
            }
        });



        frame.setResizable(false);
        frame.setDefaultCloseOperation(EXIT_ON_CLOSE);
        frame.setVisible(true);
    }


    public void windowClosing(WindowEvent e)
    {   ServerClose();
        System.exit(1);
    }
    public void windowClosed(WindowEvent e) {}
    public void windowOpened(WindowEvent e) {}
    public void windowIconified(WindowEvent e) {}
    public void windowDeiconified(WindowEvent e) {}
    public void windowActivated(WindowEvent e) {}
    public void windowDeactivated(WindowEvent e) {}
    }

The 'java.net.SocketException: socket closed' is on line 65 of the code above.

Client

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

public class Client 
    { private BufferedReader in;
    private PrintWriter out;
    private Socket socket;
    private int port;
    private String server;


    public static void main(String[] args) 
        { int portNumber = 44444;
        String serverAddress = "localhost";

        Client client = new Client(serverAddress, portNumber);

        if(!client.start())
            return;     

        }


    public Client(String server, int port)
        { this.server = server;
        this.port = port;
        }



    public boolean start() 
        { // try to connect to the server
        try {
            socket = new Socket(server, port);
        } 
        // if it failed not much I can do
        catch(Exception ec) {
            System.out.println("Error connectiong to server:" + ec);
            ec.printStackTrace();
            return false;
        }


        try
        {
            in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
            out = new PrintWriter(socket.getOutputStream(), true);;
        }
        catch (IOException eIO) {
            System.out.println("Exception creating new Input/output Streams: " + eIO);
            eIO.printStackTrace();
            return false;
        }

        // creates the Thread to listen from the server 
        new ListenFromServer().start();


        // success we inform the caller that it worked
        return true;
    }





    class ListenFromServer extends Thread 
        {
        public void run() 
            { while(true) 
                { 




                disconnect() ;
                break;
                }
            }


        }

    public void disconnect() 
        { try { 
            if(in != null) in.close();
        }
        catch(Exception e) {} // not much else I can do
        try {
            if(out != null) out.close();
        }
        catch(Exception e) {} // not much else I can do
        try{
            if(socket != null) socket.close();
        }
        catch(Exception e) {} // not much else I can do
        }

    }

Note that this is just a fragment of the whole application that I am currently building, I tried to post only what had to do with the Server-Client communication, so I deleted everything else, I'm saying this in case you see something that maybe doesn't have any purpose, I probably omitted to delete it


I see that the question got marked as duplicate, which I consider to be unfair. Firstly, in the 'similar' question, the problem was obvious, the outpot stream was closed, which closed the socket, but the socket had still been used, meanwhile, my program closes everything alltoghether and also has the CPU problem I mentioned, for which I cannnot get any answer from the so called 'similar' question.

George Cernat
  • 1,323
  • 9
  • 27
  • Duplicate, but you've omitted all the relevant code, and ignoring an `IOException` *inside* a loop is inane. – user207421 Apr 15 '17 at 11:11
  • @EJP Well, I would like to know why what I mentioned above happens, so I think that only adding the portion of the code that is relevand to the problem is the best approach. Secondly, if you could further elaborate the "ignoring an IOException inside a loop is inane" as to where and what should I do to fix it, I'd be grateful. Thirdly, I don't see how that's a duplicate, he's problem was clear, closing the output stream of a socket, then trying to use the input one, meanwhile I close all of them together. Moreover, I also got the CPU problem, which is not solved in his question. – George Cernat Apr 15 '17 at 11:15
  • It happens because you are closing the socket and then continuing to use it and ignoring the exception that tells you so. Poor effort all round. As to 'ignoring an `IOException` inside a loop', the solution is to *find* the place where you are ignoring an `IOException` inside a loop, and *stop* doing so. It's up there in black and white. I found it: so can you. You wrote it, after all. – user207421 Apr 15 '17 at 12:52

1 Answers1

2

The high CPU utilization is because your client threads aren't doing anything else besides burning the CPU with their empty loops. As for the SocketException, it works as planned, so catch it and handle it.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • Well, a client is supposed to only run for 1 second before it automatically disconnects, which it does. Shouldn't the cpu then be 'freed'/? Also, for the "socket exception", you mean to say that there isn't necessarly something wrong with the code, it's just something that it's natural to happen? – George Cernat Apr 15 '17 at 11:01
  • And in the full program, the client also has an interface with several functions, and as I said, if I let a client be connected for more than 30 seconds, the CPU will be at 100%. – George Cernat Apr 15 '17 at 11:05
  • 1
    There's a lot wrong with the code, but the socket exception is just the normal result of attempting to use a closed socket. High CPU usage is the normal result of doing busy waiting. I recommend dropping all of the code, finding some proper tutorials and then doing it again correctly. – Kayaman Apr 15 '17 at 11:34
  • It's particularly useful when someone points out that there's a lot wrong with your code, but then proceeds to not let you know what that is. – George Cernat Apr 15 '17 at 11:43
  • 1
    It's not relevant. You've written (or copied) a large amount of poor quality code. I'm not going to be your free private tutor and explain all the mistakes you've made. I advised you to find a good tutorial, because they also explain things well. – Kayaman Apr 15 '17 at 11:48
  • I did write that, after watching a couple of videos and reading some tutorials. But, best practice is actually when you write the code yourself, and that's what I came up with, good or bad, however, I can't check the tutorials to see what I did wrong, that's why I came here. Anyway, I thank you for taking the time to answer my question and comments and I wish you a Happy Easter! – George Cernat Apr 15 '17 at 11:56
  • 1
    Happy Easter to you too. – Kayaman Apr 15 '17 at 11:58