-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
No exception while using PrintWriter and outputstream #93
Comments
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. |
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
That doesn't help raising errors when the connection gets closed while in use, though. |
So this might be part of the problem: jnr-unixsocket/src/main/java/jnr/unixsocket/UnixSocketChannel.java Lines 157 to 162 in 1650e10
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. |
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 :
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 :
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.
|
Thank you for the example! This should be sufficient for me to reproduce the problem locally. |
Findings from debugging the example client.
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. |
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. |
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);
}
}
|
This did not happen in 0.39.9 due to other priorities. |
Hi,
When I use a wrapper PrintWriter over a UnixSocketChannel OutputStream, no exception is raised when I try to println(String) on the PrintWriter object while the socket is actually remotely closed.
To reproduce the issue, you need a simple client/server with a UnixSocketChanel.
Then a PrintWriter with :
PrintWriter w = new PrintWriter(Channels.newOutputStream(channel));
If you close the socket on the server side and then try to use the PrintWriter afterwards, no exception is raised.
I've seen that (PrintWriter)w.checkError() returns actually true, which shows the error. But I think a proper "SocketClosed" exception or something like that would be nice.
Thank you for your help
The text was updated successfully, but these errors were encountered: