MultiThreading, Java Server/Client Project

Question

Hi I'm currently working on a Server/ Client program in Java, where a client connects to the server and sends a number to the server, the server then adds this number to a global variable and returns the global variable. The problem I am having is when I have multiple clients one client doesn't work while the other does, the client that was created at a later stage works. I have created separate threads for each client , and works to a certain degree.

This is the snippet of my server class that creates the new thread

public static Integer globalSum = 0;

public static void main(String[] args) throws IOException
{

    ServerSocket server = new ServerSocket(8888);
    Socket s;

    System.out.println("Server running. Waiting for a client to connect...");

    while(true) {

        s = server.accept();
        ThreadHandler th=new ThreadHandler(s);
        Thread t=new Thread(th);
        t.start();
        System.out.println("Client is now connected");

    }
}

Here is my client class

public class Client2 {

    public static void main(String[] args) throws IOException {
        Socket s = new Socket("localhost", 8888);
        InputStream instream = s.getInputStream();
        OutputStream outstream = s.getOutputStream();
        Scanner in = new Scanner(instream);
        Scanner input= new Scanner(System.in);
        PrintWriter out = new PrintWriter(outstream);

        for(int i=0;i<5;i++){
            System.out.println("Please enter a value "+(i+1));
            String response=input.nextLine();
            String request="SUBMIT"+" "+response+"\n";
            out.print(request);
            out.flush();
            System.out.println(in.nextLine());
        }

        out.print("QUIT\n");
        out.flush();

        s.close();
    }

}

And here is my ThreadHandler class

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.ArrayList;
import java.util.Scanner;

public class ThreadHandler implements Runnable {

    private Socket s;
    private Scanner in;
    private static PrintWriter out;
    ServerMultipleClients serverInstance;

    public ThreadHandler(Socket s)
    {
        //this.serverInstance=serverInstance;
        this.s=s;

    }

    public void run() {
        try {
            try {

                in = new Scanner(s.getInputStream());
                out = new PrintWriter(s.getOutputStream());

                doService(); // the actual service

            } finally {
                s.close();
            }

        } catch (IOException e) {
            System.err.println(e);
        }
        return;
    }


    public void doService() throws IOException{

        String request;

        while(true){

            request=in.next();
            String arrayString[] = request.split("\\s+") ;
            if(request.equals("SUBMIT")){
                String value2=in.next();
                handle(value2);
            }
            else if (request.equals("QUIT")){
                break;
            }
            else if(!request.equals("SUBMIT")||!request.equals("QUIT")){
                System.out.println("ERROR in input");
                break;
            }

        }

    }


    public void handle(String value1){

        if(isInt(value1)==true){

            int valueInt=Integer.parseInt(value1);

            synchronized(serverInstance.globalSum) {

                if((valueInt+serverInstance.globalSum)<Integer.MAX_VALUE){
                    ServerMultipleClients.globalSum=ServerMultipleClients.globalSum+valueInt;
                    out.println("OK");
                    out.flush();
                    System.out.println("New Value for Global Sum is: "+ServerMultipleClients.globalSum);
                }
                else if((valueInt+ServerMultipleClients.globalSum)>=Integer.MAX_VALUE){
                    out.println("ERROR");
                    System.out.println("Global Sum is unchanged, its value is: "+ServerMultipleClients.globalSum);
                }
                else{
                    out.println("ERROR");
                    System.out.println("Global Sum is unchanged, its value is: "+ServerMultipleClients.globalSum);
                }

            }
        }
    }

    public boolean isInt( String input ) {
        try {
            Integer.parseInt( input );
            return true;
        }
        catch( Exception e ) {
            return false;
        }
    }

}

The problem is that when I run this, it works for each client that is run at a later stage to a previous client, going back to the previous client means that that client crashes.

Answer 1

Maybe this is not the answer to your question, but I see something that looks like a big (and very common) mistake.

I don't see the declaration of ServerInstance.globalSum, but I can guess, it's an Integer. Right?

Each time you assign it, you are creating a new object:

ServerMultipleClients.globalSum=ServerMultipleClients.globalSum+valueInt;

That means, when your code synchronizes on ServerMultipleClients.globalSum, each thread likely is synchronizing on a different Integer object. That means, that any number of threads can get into your "synchronized" block at the same time.

Whenever you write synchronized(foo), you probably want foo to be a final variable. E.g.,

private final Object foo = new Object();

That way, you can be absolutely sure that different threads will not synchronize on different instances.

Also Note! If you use synchronized(foo) to protect static data, then foo itself must be static.

Answer 2

1. What kind of the error do you have?
2. Why PrintWriter out; is static?

Anyway, I wold replace

public static Integer globalSum = 0;

with

AtomicInteger globalSum = new AtomicInteger();

and take the value one-time:

myValueAtThisTime =  globalSum.addAndGet(valueInt);

Answer 3

Are you sure this thing:

private static PrintWriter out;

must be static? It means that ALL the threads share your output stream!