java singleton thread safe

Question

Ok, here is my problem.

I need to create a socket program that can handle multiple connection from my client apps (lets call it apps1). I handle this using thread (so each connection was thrown into a new thread)

The problem is I can accept request from all open connection but when I want to send a response , I must send it through the latest connection only. So if I have 3 connections (con1,con2,con3) I can accept request from con1, con2 and con3 but I must send the response through con3 (assuming con3 is the latest connection)

I thought of using a singleton, with a PrintWriter parameter. So everytime there is a new connection ,they call the singleton and update the parameter and when I want to send the response, I get the PrintWriter first before sending.

Here is my Singleton class :

public class Singleton {

private static final Singleton instance = new Singleton();

PrintWriter  out;

public static Singleton getInstance() {
    return instance;
}
public Singleton ()
{
    if (instance != null) {
        throw new IllegalStateException("Already instantiated");
    }
}

public PrintWriter getPrintWriter ()
{
    return this.out;
}
public void updatePrintWriter (PrintWriter  out){
    this.out = out;
}
}

This is my main program :

public class SocketAccept{
private ServerSocket mainSocket;
private Socket clientSocket;

    public SocketAccept (int portNumber) {
        Singleton s = Singleton.getInstance();
        do {
        try {
            mainSocket = new ServerSocket(portNumber);
            clientSocket = mainSocket.accept();
            s.updatePrintWriter(new PrintWriter(clientSocket.getOutputStream(), true));
            ClientThread (clientSocket);
        } catch (IOException ex) {
            Logger.getLogger(TestClass.class.getName()).log(Level.SEVERE, null, ex);
        }
        }while (true);//need to change this into thread pool or connection pool    
    } 
}

and this is my thread that handle socket :

public class ClientThread extends Thread {

    private Socket cs;
    Singleton s = Singleton.getInstance();
    PrintWriter out;

    private String read(Socket sc) {
        String request = "";
        //read request here
        return request;
    }

    private String process(String request) {
        String response = "";
        //process request here
        return response;
    }

    public ClientThread(Socket clientSocket) {
        this.cs = clientSocket;
    }

    @Override
    public void run() {
        String requestMsg = "";
        String responseMsg = "";
        do {
            requestMsg = read(cs);// read the message

            if (requestMsg.equalsIgnoreCase("SHUTDOWN")) {
                break;
            }
            responseMsg = process(requestMsg);
            out = s.getPrintWriter();
            out.write(responseMsg);
        } while (true);
    }
}

Do I did it right? Or it is impossible to do it with singleton?

Thanks for the help.

Answer 1

Unfortunately, that is not a thread safe implementation of the Singleton pattern. I don't think you need one in this case either, an AtomicReference will probably work just fine. Try this:

public class SocketAccept{
    private ServerSocket mainSocket;
    private Socket clientSocket;

    private final AtomicReference<PrintWriter> printWriterHolder = new AtomicReference(null);

    public SocketAccept (int portNumber) {
        Singleton s = Singleton.getInstance();
        do {
        try {
            mainSocket = new ServerSocket(portNumber);
            clientSocket = mainSocket.accept();
            printWriterHolder.set(new PrintWriter(clientSocket.getOutputStream(), true));
            Thread clientThread = new ClientThread (clientSocket, printWriterHolder);
            clientThread.start();
        } catch (IOException ex) {
            Logger.getLogger(TestClass.class.getName()).log(Level.SEVERE, null, ex);
        }
        }while (true);//need to change this into thread pool or connection pool    
    } 
}

...

public class ClientThread extends Thread
    ...
    private final AtomicReference<PrintWriter> printWriterHolder;
    public ClientThread(Socket clientSocket, AtomicReference<PrintWriter> holder) {
        this.cs = clientSocket;
        this.printWriterHolder = holder;
    }

    @Override
    public void run() {
        String requestMsg = "";
        String responseMsg = "";
        do {
            requestMsg = read(cs);// read the message

            if (requestMsg.equalsIgnoreCase("SHUTDOWN")) {
                break;
            }
            responseMsg = process(requestMsg);
            out = printWriterHolder.get();
            out.write(responseMsg);
        } while (true);
    }
}

If you really do want to use the Singleton pattern, here is a reference from SO where the question has a good thread-safe implementation for creating the Singleton: Java Singleton Pattern

You will also need to make accessing the Singleton's state thread-safe by using synchronized, Lock, or atomic operations (AtomicInteger, AtomicReference, etc..) as necessary.