No exception while using PrintWriter and outputstream about jnr-unixsocket

[headius]

An exception would indeed be appropriate here. Can you put together an example test case that shows the problem? I think it would be fine to use jnr-unixsocket as both the client and server for such a test.

[headius]

I notice that our logic for getting the OutputStream does check for connectedness:

jnr-unixsocket/src/main/java/jnr/unixsocket/UnixSocket.java

Lines 123 to 130 in 1650e10

	@Override
	public OutputStream getOutputStream() throws IOException {
	if (chan.isConnected()) {
	return out;
	} else {
	throw new IOException("not connected");
	}
	}

That doesn't help raising errors when the connection gets closed while in use, though.

[headius]

So this might be part of the problem:

jnr-unixsocket/src/main/java/jnr/unixsocket/UnixSocketChannel.java

Lines 157 to 162 in 1650e10

	public boolean isConnected() {
	stateLock.readLock().lock();
	boolean result = state == State.CONNECTED;
	stateLock.readLock().unlock();
	return result;
	}

As you see here, we do not check the associate file descriptor to see if it is still connected, and instead assume that if our internal logic says we are connected, we are connected.

Checking the file descriptor for openness or connectedness every time we read or write may be unreasonable, though, so we should also handle errors during read or write by raising an appropriate error (and if it is a closed socket, we probably should mark ourselves closed too).

It would be helpful if you can provide a complete example that shows what you expect to happen.

[peacand]

Thank you for your first analysis @headius. Unfortunately I cannot help you about the implementation, I'm not a Java guy 😊

I'm actually only talking about the client part, which looks like in my tests :

public class UnixClient {
    public static void main(String[] args) throws IOException, InterruptedException {
        String next = "Init";
        BufferedReader consoleReader = new BufferedReader(new InputStreamReader(System.in));
        java.io.File path = new java.io.File("/tmp/unix.sock");
        UnixSocketAddress address = new UnixSocketAddress(path);
        UnixSocketChannel channel = null;
        try {
            channel = UnixSocketChannel.open(address);
        } catch (Exception e) {
            System.out.println("Unix sock connection failed");
            return;
        }
        System.out.println("connected to " + channel.getRemoteSocketAddress());
        // output
        OutputStream os = Channels.newOutputStream(channel);
        PrintWriter w = new PrintWriter(os);
        // input
        InputStreamReader r = new InputStreamReader(Channels.newInputStream(channel));
        BufferedReader in = new BufferedReader(r);

        while (next != null) {
            w.println("Test");
            w.flush();
            System.out.println("Write error: " + w.checkError());
            String result = in.readLine();
            System.out.println("read from server: " + result);
            System.out.println("Continue ?");
            next = consoleReader.readLine();
        }
        System.exit(0);
    }
}

The server part can be anything. I've tested with a Python code but it can be any server listening on Unix socket and replying to messages on that socket. You can even mock a simple server with the tool Socat :

socat UNIX-LISTEN:/tmp/unix.sock -

Basically if you start the server and the client, everything is fine. Then if you shutdown the server, the client can continue to call the functions readline() and println() on the input/output stream without any exception while the underlying socket is actually closed. I would have expected to get an exception while trying to red/write on a stream where the underlying FD is actually closed.

[headius]

Thank you for the example! This should be sufficient for me to reproduce the problem locally.

[headius]

Findings from debugging the example client.

• Confirmed that no errors are raised and it keeps reading and writing nothing without errors.
• When blocked on read and the server is closed, the native read call returns 0. This is interpreted as EOF and returned into the Java classes as -1, the Java indicator of EOF. A -1 here would indicate an error, but apparently the server going away during a blocking read is not considered an error state. I will not dig into the read side for now.
• When attempting to write after the server has been terminated, the return value from the native write call is indeed -1 and the errno is EPIPE. This is raised as a NativeException from the Common class. The exception bubbles out until it reaches code in the JDK PrintWriter, which swallows the error and sets the "trouble" field to true. This is what you get when you call checkError.
• The JavaDoc for PrintWriter includes this section:

  Methods in this class never throw I/O exceptions, although some of its
  constructors may. The client may inquire as to whether any errors have
  occurred by invoking checkError().

Long story short, it seems like the jnr-unixsocket classes are actually working right, but PrintWriter apparently silences most errors that bubble out from the stream it wraps.

[headius]

Actually there may be one thing we can do better here: close the channel when we get an EPIPE from read or write.

I tried to find some evidence of this in the JDK classes, like SocketChannelImpl, but the logic is rather tangled. There are checks to see if the last operation completed successfully, but they don't appear to change the open/closed status of the channel.

Regardless, I think you would still see the same behavior if we marked the channel as closed, since per doco the PrintWriter class will never raise an IO exception.

[headius]

Still not sure the best way to handle this. Even the Java classes seem to trip a bit over closed pipes and sockets.

I was playing with this patch, perhaps it gets us close enough to handling this?

diff --git a/src/main/java/jnr/unixsocket/impl/Common.java b/src/main/java/jnr/unixsocket/impl/Common.java
index 9b1dd55..72e8042 100644
--- a/src/main/java/jnr/unixsocket/impl/Common.java
+++ b/src/main/java/jnr/unixsocket/impl/Common.java
@@ -20,6 +20,7 @@ package jnr.unixsocket.impl;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.channels.ClosedChannelException;
 
 import jnr.constants.platform.Errno;
 import jnr.enxio.channels.Native;
@@ -115,8 +116,12 @@ final class Common {
                 case EWOULDBLOCK:
                     src.position(src.position()-r);
                     return 0;
-            default:
-                throw new NativeException(Native.getLastErrorString(), lastError);
+
+                case EPIPE:
+                    throw new ClosedChannelException();
+
+                default:
+                    throw new NativeException(Native.getLastErrorString(), lastError);
             }
         }
 

[headius]

This did not happen in 0.39.9 due to other priorities.