Fixing too many open files Exception(I am using try-catch-finally)

Question

I have web service written on JAVA (version 1.8) which connects HSM and sends/receives data via socket. My application is deployed on Apache Tomcat/8.5.14 on linux.

Although I'm closing socket connection properly I have

java.net.SocketException: Too many open files

and here is myclass

public class myClass implements AutoCloseable {
    Socket socket;
    DataInputStream in;
    DataOutputStream out;

    public myClass(String ip, int port) throws Exception {
        try {
            socket = new Socket(ip, port);
            in = new DataInputStream(new BufferedInputStream(socket.getInputStream()));
            out = new DataOutputStream(new BufferedOutputStream(socket.getOutputStream()));
        } catch (IOException e) {
            throw new Exception("Connecting to HSM failed" + e);
        }
    }       

   public String sendCommandToHsm(String command) throws IOException {
        out.writeUTF(command);
        out.flush();
        return in.readUTF();
    }

    @Override
    public void close() {
        if (socket != null && !socket.isClosed()) {
            try {
                socket.close();
            } catch (IOException e) {
                lgg.info("Closing of socket failed", e);
            }
        }

        if (in != null) {
            try {
                in.close();
            } catch (IOException e) {
                lgg.info("Closing of inputStream failed", e);
            }
        }

        if (out != null) {
            try {
                out.close();
            } catch (IOException e) {
                lgg.info("Closing of outputStream failed", e);
            }
        }
    }

}

Here is using of my class

try (MyClass myClass = new MyClass(ip, port);) {
      myClass.sendCommandToHsm("my command");
 }

I increased maximum open files limit on server from default value(1024) to 8192 and few times later the same Exception occurred again.

I'm thinking about creating Socket Connection Pool, is it good idea?

Can you suggest any other solutions?

Answer 1

Although I'm closing socket connection properly ...

It looks like you are, but I think there are a couple of problems. (I don't know that these are the cause of your leak, but the first one is a plausible explanation.)

Problem 1.

public myClass(String ip, int port) throws Exception {
    try {
        socket = new Socket(ip, port);
        in = new DataInputStream(new BufferedInputStream(socket.getInputStream()));
        out = new DataOutputStream(new BufferedOutputStream(socket.getOutputStream()));
    } catch (IOException e) {
        throw new Exception("Connecting to HSM failed" + e);
    }
}  

If an exception is thrown during setup of the streams, then the socket will be leaked.

Problem 2.

public void close() {
    if (socket != null && !socket.isClosed()) {
        try {
            socket.close();
        } catch (IOException e) {
            lgg.info("Closing of socket failed", e);
        }
    }

    if (in != null) {
        try {
            in.close();
        } catch (IOException e) {
            lgg.info("Closing of inputStream failed", e);
        }
    }

    if (out != null) {
        try {
            out.close();
        } catch (IOException e) {
            lgg.info("Closing of outputStream failed", e);
        }
    }
}

You are closing in the wrong order. You should¹ close in and out before closing socket. In particular, if out has buffered data, then closing out will attempt to flush ... which will fail if you have closed socket already.

Also if socket.close() or in.close() fails for some other reason than an IOException, then the subsequent closes will be skipped. So you should use a finally here.

Also the isClosed() call is redundant. Calling close() on a resource that is already closed should do nothing. This is part of the contract of close().

Finally, calling close() on a socket should² automatically close the low-level file descriptors beneath in and out. So it is arguably best to just do this:

public void close() {
    if (socket != null) {
        try {
            socket.close();
        } catch (IOException e) {
            lgg.info("Closing of socket failed", e);
        }
    }
}

---

If this doesn't fix your leaks, I suggest that you use netstat and lsof to try to find out if the leakage is open files or open sockets.

---

I'm thinking about creating Socket Connection Pool, is it good idea?

Yes ... if you can find an existing (well designed and tested) library that meets your requirements. Implementing a reliable pool from scratch is not trivial.

But note:

1. An incorrectly implemented (or used) pool can leak file descriptors.
2. The server-side needs to be able to cope with a series of requests / replied on the same connection.
3. If you have too many simultaneous open connections to different places, then the pool needs a way to close some of them ...

---

^{1 - It is debatable whether you should close out before in. On the one hand, closing out flushes outstanding data to the server. On the other hand, by the time that myClass.close() has been called there won't be anything reading the server's response. And besides, the sendCommandToHsm method flushes ... so there shouldn't be any outstanding data.}

^{2 - The javadoc for Socket.close() says: "Closing this socket will also close the socket's InputStream and OutputStream."}