2
\$\begingroup\$

I have 3 classes: Server, Client, RequestHandler It's about reading out commands and executing them What the programm does: 1) The Server starts, opens a ServerSocket and starts listening for Clients on that Socket. 2)Several Clients start and try to connect to the socket. 3)If a client succesfully connects to the Socket(only 3 at a time are allowed to connect, using ExecutorService with a Fixed Pool for that), the Server passes the Clients request to the RequestHandler, which processes it 4)THe server sends the processed message back to the client 5)If the message was SHUTDOWN, the Server stops accepting new clients immideately and the one's still active are given a certain period of time(here:10 seconds) to interact with the Server, before they are being shutdown as well.

It took me a whole week every day after work to programm this and finally make it work. I don't see a better apporach to doing it, but there MUST be. My whole code is so ugly.

Things that bug me: To avoid the Server being stuck on the accept() call, since my I/O are blocking functions, I had to find a way to sneak around this. My solution to this was to implement a shutdown method with a static scope, so it can be accessed from the RequestHandler.

Having to make everything static does not feel right though. For what I understand, you should make things static if they make SENSE existing without an instance of the class being created. Having an Executor and shutting down the Server (both static here) only makes sense if the Server actually exists. So yeah, it's pretty screwed up.

Thank you in advance for all your suggestions, it is a lot of code. Here it comes:

The Server

public class MulServer_v1 {

    protected static int portNumber = 8540;
    protected static int max_Clients = 3;
    protected static boolean shutdownFlag = false;
    protected static ServerSocket serverSocket;
    protected static ExecutorService executor;
    protected static ArrayList<Socket> socketList = new ArrayList<>();

    public MulServer_v1(int portNumber, int poolSize) {
    }

    public void runServer() {

        try {
            serverSocket = new ServerSocket(portNumber);
            executor = Executors.newFixedThreadPool(max_Clients);
        } catch (IOException e) {
            System.out.println("Could not create server on specific port");
            e.printStackTrace();
        }

        while (!shutdownFlag) {
            try {
                Socket clientSocket = serverSocket.accept();
                socketList.add(clientSocket);
                executor.submit(new RequestHandler_v1(clientSocket));

            } catch (IOException e) {
                System.out.println("Couldn't accept on the Socket");
                executor.shutdown();
                e.printStackTrace();

            }

        }
        shutdownAndAwaitTermination();
    }   


    protected static void shutdownAndAwaitTermination() {
        System.out.println("Shutting down..");
        executor.shutdown(); // Disable new tasks from being submitted

        try {
            // Wait a while for existing tasks to terminate
            if (!executor.awaitTermination(10, TimeUnit.SECONDS)) {
                for (Socket s : socketList) {

                    try {
                        s.close();
                    } catch (IOException e) {
                        System.out.println("Couldn't close the socket");
                        e.printStackTrace();
                    }
                }
                executor.shutdownNow();
                // Cancel currently executing tasks
                System.out.println("komme ich hierhin?");

                // Wait a while for tasks to respond to being cancelled
                if (!executor.awaitTermination(10, TimeUnit.SECONDS))
                    System.err.println("Pool did not terminate");
            }
        } catch (InterruptedException ie) {
            // (Re-)Cancel if current thread also interrupted
            executor.shutdownNow();
            // Preserve interrupt status
            Thread.currentThread().interrupt();
        }
        try {
            serverSocket.close();
        } catch (IOException e) {
            System.out.println("Serversocket konnte nicht geschlossen werden");
            e.printStackTrace();
        }
        System.out.println("I got here!");

    }

    public static void main(String[] args) {
        MulServer_v1 server = new MulServer_v1(portNumber, max_Clients);
        server.runServer();
    }

}

The Client

public class Client_v1 {

    public static final String HOSTNAME = "localhost";
    public static final int PORTNUMBER = 8540;
    private static boolean clientClose = false;

    public static void main(String[] args) throws IOException {
        System.out.println("Client started");

        try (Socket socket = new Socket(HOSTNAME, PORTNUMBER);

                PrintWriter out = new PrintWriter(socket.getOutputStream(),
                        true);
                // InputStream test = echoSocket.getInputStream();
                BufferedReader in = new BufferedReader(new InputStreamReader(
                        socket.getInputStream()));
                BufferedReader stdIn = new BufferedReader(
                        new InputStreamReader(System.in))) {
            String userInput;

            while ((userInput = stdIn.readLine()) != null && !clientClose) {
                out.println(userInput);
                System.out.println("echo: " + in.readLine());
                // if (userInput.equals("BYE")) {
                // break;
                // }

            }
        } catch (UnknownHostException e) {
            System.err.println("Don't know about host " + HOSTNAME);
            System.exit(1);
        } catch (IOException e) {
            System.err.println("Couldn't get I/O for the connection to "
                    + HOSTNAME);
            System.exit(1);
        }

    }

    protected static void closeClient() {
        clientClose = true;

    }
}

The RequestHandler

public class RequestHandler_v1 implements Runnable {
    // private final String password = "passwort";
    private final Socket client;
    private boolean closeFlag = false;

    public RequestHandler_v1(Socket client) {
        this.client = client;
    }

    @Override
    public void run() {
        try (BufferedReader in = new BufferedReader(new InputStreamReader(
                client.getInputStream()));
                BufferedWriter writer = new BufferedWriter(
                        new OutputStreamWriter(client.getOutputStream()));) {
            System.out.println("Thread started with name:"
                    + Thread.currentThread().getName());
            String userInput;
            String serverResponse;

            while ((userInput = in.readLine()) != null) {
                serverResponse = processInput(userInput);
                System.out.println("Received message from "
                        + Thread.currentThread().getName() + " : " + userInput);
                writer.write("Sever Response : " + serverResponse);
                writer.newLine();
                writer.flush();
                if (closeFlag) {
                    Client_v1.closeClient();
                    MulServer_v1.socketList.remove(client);
                    client.close();
                    MulServer_v1.shutdownAndAwaitTermination();
                }
            }
        } catch (IOException e) {
            System.out.println("I/O exception: " + e);
        } catch (Exception ex) {
            System.out.println("Exception in Thread Run. Exception : " + ex);
        }
    }

    public String processInput(String input) {
        boolean commandFound = false;
        String output = "";
        try {
            if (input.getBytes("UTF-8").length > 255)
                output = "Max string length exceeded";
        } catch (UnsupportedEncodingException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        Pattern allPattern = Pattern
                .compile("(?<lower>^LOWERCASE\\s.+)|(?<upper>^UPPERCASE\\s.+)|(?<reverse>^REVERSE\\s.+)|(?<bye>^BYE)|(?<shutdown>^SHUTDOWN passwort)");

        Matcher allMatcher = allPattern.matcher(input);
        if (allMatcher.find()) {
            String lower = allMatcher.group("lower");
            String upper = allMatcher.group("upper");
            String reverse = allMatcher.group("reverse");
            String bye = allMatcher.group("bye");
            String shutdown = allMatcher.group("shutdown");
            commandFound = true;
            if (lower != null) {
                output = lower.substring(10).toLowerCase();
            }
            if (upper != null) {
                output = upper.substring(10).toUpperCase();
            }
            if (reverse != null) {
                output = new StringBuilder(reverse.substring(8)).reverse()
                        .toString();
            }
            if (bye != null) {
                output = "BYE";
                closeFlag = true;
            }
            if (shutdown != null) {
                output = "SHUTDOWN";
                MulServer_v1.shutdownFlag = true;
                closeFlag = true;
            }
        } else {
            commandFound = false;
            output = "UNKNOWN COMMAND";
        }

        if (commandFound) {
            output = "OK ".concat(output);
        } else {
            output = "ERROR ".concat(output);

        }
        return output;

    }
}
\$\endgroup\$
2
\$\begingroup\$

I agree that going static across the board isn't the right thing to do. An alternative approach might be to pass the server instance into your RequestHandler and call shutdown on that:

public RequestHandler_v1(Socket client, Server server) {
    this.client = client;
    this.server = server;
}

and then:

executor.submit(new RequestHandler_v1(clientSocket, this));

and

server.shutdownAndAwaitTermination();

On an unrelated note, something else I noticed is a call to MulServer_v1.socketList.remove(client);. You should avoid using properties directly from other classes, and so you should do this via a method (say, server.cleanupClient(client);).

\$\endgroup\$
  • \$\begingroup\$ Hmm. I tried your approach. But somehow this has affected the logic. Now I can let a new Client connect to the server after the shutdown has been initiated \$\endgroup\$ – InDaPond Oct 1 '16 at 10:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.