1

The below program acts as TCP client and uses NIO to open socket to a remote server, as below

private Selector itsSelector;
private SocketChannel itsChannel;

public boolean getConnection(Selector selector, String host, int port)
{
    try
    {
        itsSelector = selector;
        itsChannel = SocketChannel.open();
        itsChannel.configureBlocking(false);
        itsChannel.register(itsSelector, SelectionKey.OP_CONNECT);
        itsChannel.connect(new InetSocketAddress(host, port));
        if (itsChannel.isConnectionPending())
        {
            while (!itsChannel.finishConnect())
            {
                // waiting until connection is finished
            }
        }
        itsChannel.register(itsSelector, SelectionKey.OP_WRITE);
        return (itsChannel != null);
    }
    catch (IOException ex)
    {
        close();
        if(ex instanceof ConnectException)
        {
            LOGGER.log(Level.WARNING, "The remoteserver cannot be reached");
        }
    }
}

public void close()
{
    try
    {
        if (itsChannel != null)
        {
            itsChannel.close();
            itsChannel.socket().close();
            itsSelector.selectNow();
        }
}   
    catch (IOException e)
    {
        LOGGER.log(Level.WARNING, "Connection cannot be closed");
    }
}

This program runs on Red Hat Enterprise Linux Server release 6.2 (Santiago) When number of concurrent sockets are in establishment phase, file descriptor limit reaches a max value and I see below exception while trying to establish more socket connections.

java.net.SocketException: Too many open files
               at java.net.PlainSocketImpl.socketAccept(Native Method)
               at java.net.PlainSocketImpl.accept(PlainSocketImpl.java:408)

This happens only when the remote Node is down, and while it is up, all is fine. When the remote TCP server is down, below exception is thrown as is handled as IOException in the above code

java.net.ConnectException: Connection refused: no further information
    at sun.nio.ch.SocketChannelImpl.checkConnect(Native Method)
    at sun.nio.ch.SocketChannelImpl.finishConnect(Unknown Source)

Is there any way to forcefully close the underlying file descriptor in this case. Thanks in advance for all the help.

  • 1
    Why would you configure blocking false and then block anyway? – Neijwiert Mar 07 '16 at 07:39
  • According to the Javadoc, it should be closed automatically by the nio library: "If a connection attempt fails, that is, if an invocation of this method throws a checked exception, then the channel will be closed.". – Erwin Bolwidt Mar 07 '16 at 08:43
  • @ErwinBolwidt To clarify, that text comes from the description of `finishConnect()`, which shouldn't even be being called here. – user207421 Mar 07 '16 at 08:58
  • Even after explicitly closing the SocketChannel(itsChannel.close) and itsChannel.isOpen() returning false , the underlying file descriptor is not being closed – Supriya Kulkarni Mar 07 '16 at 17:55
  • @SupriyaKulkarni You don't know that. It could be a descriptor for something else, for example a Selector. – user207421 Mar 07 '16 at 20:52
  • How do we close the selector, when we intend to close the connection ? itsSelector.close(); ? – Supriya Kulkarni Mar 08 '16 at 05:46
  • @SupriyaKulkarni Of course, but you don't have to close the selector at all. You can just keep using the same one. If you're creating mutiple Selectors, say one per connect, that's the problem. Don't. – user207421 Mar 09 '16 at 01:03

1 Answers1

4
private Selector itsSelector;

I cannot see the point of this declaration. You can always get the selector the channel is registered with, if you need it, which you never do. Possibly you are leaking Selectors?

itsChannel.configureBlocking(false);
itsChannel.register(itsSelector, SelectionKey.OP_CONNECT);

Here you are registering for OP_CONNECT but never making the slightest use of the facility.

itsChannel.connect(new InetSocketAddress(host, port));

Here you are starting a pending connection.

if (itsChannel.isConnectionPending())

It is. You just started it. The test is pointless.

{
    while (!itsChannel.finishConnect())
    {
        // waiting until connection is finished
    }
}

This is just a complete waste of time and space. If you don't want to use the selector to detect when OP_CONNECT fires, you should call connect() before setting the channel to non-blocking, and get rid of this pointless test and loop.

    itsChannel.register(itsSelector, SelectionKey.OP_WRITE);
    return (itsChannel != null);

itsChannel cannot possibly be null at this point. The test is pointless. You would be better off allowing the IOExceptions that can arise to propagate out of this method, so that the caller can get some idea of the failure mode. That also places the onus on the caller to close on any exception, not just the ones you're catching here.

catch (IOException ex)
{
    close();
    if(ex instanceof ConnectException)
    {
        LOGGER.log(Level.WARNING, "The remoteserver cannot be reached");
    }
}

See above. Remove all this. If you want to distinguish ConnectException from the other IOExceptions, catch it, separately. And you are forgetting to log anything that isn't a ConnectException.

public void close()
{
    try
    {
        if (itsChannel != null)
        {
            itsChannel.close();
            itsChannel.socket().close();
            itsSelector.selectNow();

The second close() call is pointless, as the channel is already closed.

catch (IOException e)
{
    LOGGER.log(Level.WARNING, "Connection cannot be closed");
}

I'm glad to see you finally logged an IOException, but you're not likely to get any here.

Don't write code like this.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • "I'm glad to see you finally logged an IOException, but you're not likely to get any here." Not likely, but still posible, however if that fails your program should abort :). Good answer. – Neijwiert Mar 07 '16 at 09:17
  • The root cause of file descriptor leakage in my case was the Selector not being closed, when connection fails to establish or the remote server is down. When the closed the selector in catch block of getConnection method, the issue is solved – Supriya Kulkarni Mar 08 '16 at 14:21
  • @SupriyaKulkarni That's a poor solution. You're closing a Selector that was provided by somebody else. The actual problem is the somebody else creating endless Selectors. You only need one for the life of the program. – user207421 Mar 09 '16 at 01:02
  • Indeed we use a single selector to initiate a TCP client connection towards Server. And once successful, we keep on using the same. Only when the previous request to Server fails (indicating server is down), as part of failover logic we open new connection towards one of the two servers(by using a new Selector) , and this happens whenever we get a new call . That's the reason of Selector leakage only when TCP Server is down. – Supriya Kulkarni Mar 09 '16 at 05:22
  • 1
    Opening a new connection does not require creating a new Selector. Nothing does. Don't. – user207421 Sep 22 '17 at 05:46