My Own ThreadPool implementation

I tried to make a Server using Java with a Thread Pool. Any review is welcome:

/* User: koray@tugay.biz Date: 21/02/15 Time: 21:12 */

import java.io.IOException;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.Stack;

public class MyConnectionAccepter {

    private Stack mySocketThreads = new Stack();
    private volatile int currentNumberOfConnections = 0;

    public MyConnectionAccepter() {

        MySocketThread mySocketThreadOne = new MySocketThread(this);
        MySocketThread mySocketThreadTwo = new MySocketThread(this);

        mySocketThreadOne.setDaemon(true);
        mySocketThreadTwo.setDaemon(true);

        mySocketThreadOne.start();
        mySocketThreadTwo.start();

        mySocketThreads.push(mySocketThreadOne);
        mySocketThreads.push(mySocketThreadTwo);

    }

    public void start() throws IOException {

        ServerSocket serverSocket = new ServerSocket(8888);

        while (true) {
            while (currentNumberOfConnections < 2) {
                Socket accept = serverSocket.accept();
                MySocketThread mySocketThread = mySocketThreads.pop();
                mySocketThread.setSocket(accept);
                currentNumberOfConnections++;
            }
        }

    }

    public void informIAmDone(MySocketThread mySocketThread) {
        mySocketThreads.push(mySocketThread);
        currentNumberOfConnections--;
    }
}

and

```
/ User: koray@tugay.biz Date: 21/02/15 Time: 21:04 /

import java.io.IOException;
import java.net.Socket;
import java.util.Scanner;

public class MySocketThread extends Thread {

private volatile Socket socket;
MyConnectionAccepter myConnectionAccepter;

public MySocketThread(MyConnectionAccepter myConnectionAccepter) {
this.myConnectionAccepter = myConnectionAccepter;
}

@Override
public synchronized void run() {
serve();
}

public void setSocket(Socket socket) {
this.socket = socket;
}

pub

Generic code review

This is know as a busy loop:

while(socket == null) {

    }

Not a good idea in any language.

Do you hear your fans spin up when your threads start executing this loop?

You should probably block the thread until the condition becomes true: See is there a 'block until condition becomes true' function in java?

Recursively calling your own function is not a good idea:

public void serve() {
    .. DO WORK ITEM 1
    serve();
}

That would not explode immediately. But in a production system that is running a long time that will eventually fill up the stack and cause the application to crash. People will spend years trying to work out why it crashed (because its not obvious that this is a slow and steady memory leak that is undetectable).

Why not put that in a loop?

public synchronized void run() {
    while(!finished) {
        serve();
    }
     // remove the recursive call inside serve()
}

This is another hidden busy loop:

while (currentNumberOfConnections < 2) {
            Socket accept = serverSocket.accept();
            MySocketThread mySocketThread = mySocketThreads.pop();
            mySocketThread.setSocket(accept);
            currentNumberOfConnections++;
        }

When you have two active connections then this will spin like mad and just eat CPU resources. Put another block in there so that the thread is paused until your worker threads can keep going
Design Review:

I think your socket and your threads are to tightly coupled. I would rather separate them and make them independent of each other.

Have your socket class accepts incoming requests and for each request create a work item in a queue. That way it keeps accepting requests and does not need to drop any. If the threads get there two slowly then the client will close the connection and your thread will see this when it tries to service the client rquest.

One the other side make your threads collect work items from the queue (mentioned above). You then can add more threads without the socket class knowing about any of the threads (they just know about the queue).

StackExchange Code Review Q#82193, answer score: 4