11

I have this damnable structure:

public void run() {

        try {
            if (!portField.getText().equals("")) {              
                String p = portField.getText();
                CharSequence numbers = "0123456789";

            btnRun.setEnabled(false);

            if (p.contains(numbers)) {
                ServerSocket listener = new ServerSocket(Integer.parseInt(p));

                while (true) {
                    Socket socket = listener.accept();
                    try {
                        PrintWriter out = new PrintWriter(socket.getOutputStream(), true);
                        out.println("Hi there, human.");    
                    } finally {
                        socket.close(); 
                    }
                }} else {
                    JOptionPane.showMessageDialog(null, "Only numbers are allowed.");
                }
            }

        } catch (NumberFormatException | HeadlessException | IOException e) {
            e.printStackTrace();
        } 
    }

As you can see, I need to close the listener exactly as I did with the socket. The problem is that if I try to do so after the loop the code will be "unreachable", and if I try to declare a field anywhere for the ServerSocket, I get a NullPointerException. I don't want to close the ServerSocket together with the socket/client because I want to make new connections.

So that's my question:

It's REALLY necessary to close the ServerSocket in this case? The ServerSocket closes itself by its own when the software is shut down? (System.exit(0)). If the ServerSocket continues to run when I close the software just because I have not closed it, then I have a problem, because I can't reach that bloody piece of code to close it :).

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Ericson Willians
  • 7,606
  • 11
  • 63
  • 114

3 Answers3

11

Yes. While destroying references to the socket may cause the garbage collector to finalize it, that does not specify that it will be closed. This is implementation-specific in many cases, and can sometimes derail relative to design due to performance or even minute bugs that are hard to track.

Keeping a reference anywhere is a sure bet that it won't be, even with WeakReferences.

Now the OS has a limited(due to its design) number of sockets it can give. As you open more and more sockets, the OS comes under pressure and eventually runs out of sockets, causing the failure of either Java or another program. In addition, depeding on socket options you or defaults may set, this socket may send keepalives, exhausting resources on the other endpoint as well.

On exit, it is closed by the socket registering a shutdown action in its constructor to close it, and/or the OS's automatic cleanup.

You should never rely on OS/JVM behavior or finalization to close your sockets, especially if you have multiple sockets, even if you don't plan to use all of them at the same time.

nanofarad
  • 40,330
  • 4
  • 86
  • 117
8

Yes, it's necessary to release any finite resources, because otherwise your app will make other processes on the host starve for resources and won't be able to sustain in a long run

Here's how I do such task:

public void run() {
    ServerSocket serverSocket = null;
    try {
        serverSocket = ... // init here
    } catch (...) {

    } finally {
        if (serverSocket != null) {
            try {
                serverSocket.close();
            } catch (IOException e) {
                // log error just in case
            }
        }
    }
}

Additionally, moving GUI code to other class would be a nice idea, to keep things clean.

Victor Sorokin
  • 11,878
  • 2
  • 35
  • 51
  • I'd suggest using Java 7 code here, specifically the try-with-resources: `try (ServerSocket serverSocket = ...) { ... } catch (...) { ... }` which effectively does the same thing, in many fewer lines. – FThompson Jun 30 '13 at 02:07
4

One answer is that you really ought to close() resources ... even if it is not strictly necessary.

Why?

Because, the circumstances that (hypothetically) mean that it is not strictly necessary could change!

And ... lets face it ... it is easy to write code to automatically close a server socket.

But read on ...

It's REALLY necessary to close the ServerSocket in this case?

It depends.

  • If you know for certain that run() is only ever going to run once and then the application is always going to completely shut down (modulo caveats below), then not closing the ServerSocket won't cause any actual harm.

  • Otherwise, failure to close() could cause harm.

The "harm" could take a couple of forms:

  • The server socket could hold a "lock" on a port, preventing another instance of the application from listening on that port (depending on the OS).

  • The server socket could be a limited resource, and if the run() method is called multiple times (without close()-ing) the application could "grab" all available instances of that resource, preventing other applications from acquiring an instance.

The harm could also be inflicted on the application instance itself, if run() is called multiple times.

The ServerSocket closes itself by its own when the software is shut down?

The ServerSocket doesn't "close itself" unless it is garbage collected and finalized. And you can't rely on either happening when a JVM exits. But in a broader sense, the underlying server socket resource is typically closed (released) automatically when the JVM exits.

But the complete answer actually depends on what you mean by "the software is shut down", and also on the nature of the platform. For example:

  • If shutdown means that the JVM exits completely, and the corresponding process that "owns" the underlying server socket (from the OS perspective) also exits, then the OS will close that socket ... at least on modern Unix / Linux / Windows platforms. (Note that Android is Linux under the hood)

  • If "the software" is something like a webapp running in a web container, the "shutdown" can mean something different to JVM exit, and the ServerSocket could continue to exist (and not be closed) after the shutdown.

  • If the JVM is embedded in something else (e.g. in a large C / C++ application) and that something else doesn't exit, then the OS won't know to release the underlying server socket.

  • If you are running on platform where the OS doesn't provide the same level of process separation / resource management as on Unix / Linux / (modern) Windows, then the underlying server socket may not be closed by the OS on process exit. (This scenario might apply for a Java implementation on an embedded system ...)

At any rate, it is not hard to do the right thing. In Java 7 and later:

public void run() {
    String p = portField.getText().trim();
    if (p.isEmpty()) {
        return;
    }
    btnRun.setEnabled(false);
    try (ServerSocket listener = new ServerSocket(Integer.parseInt(p))) {
        while (true) {
            try (Socket socket = listener.accept();
                 PrintWriter out = new PrintWriter(
                         socket.getOutputStream(), true)) {
                out.println("Hi there, human.");    
            }
        }
    } catch (NumberFormatException e) {
        JOptionPane.showMessageDialog(null, "Only numbers are allowed.");
    } catch (HeadlessException | IOException e) {
        e.printStackTrace();
    } finally {
        btnRun.setEnabled(true); // ... possibly
    }
}

(Though, I don't see the point of catching HeadlessException in what appears to be an action called from a button listener.)

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216