From 5c51df50a51db38dd16dd461c9b961bc2e72ddba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Kohlschu=CC=88tter?= Date: Mon, 29 Jan 2024 17:15:04 +0100 Subject: [PATCH] common: FileDescriptorCast: Update blocking state as necessary Previously, casting a FileDescriptor to Socket would not reset its (cached) blocking state to "blocking" if it was configured non-blocking, which may lead to unexpected behavior. Check the "blocking" state upon casting and set it accordingly when a Socket/DatagramSocket/ServerSocket is required (since they are expected to be blocking by default). When casting to a SocketChannel/DatagramChannel/ServerSocketChannel, adjust the Java-cached "blocking" state to the actual blocking state of the native socket. Moreover, add support to declare "we don't know the state" (like in Windows, which is currently not supported for casting, though), and ensure we get the cached state and the native state back in sync. https://github.com/kohlschutter/junixsocket/issues/151 --- .../newsclub/net/unix/AFDatagramChannel.java | 2 +- .../net/unix/AFServerSocketChannel.java | 2 +- .../newsclub/net/unix/AFSocketChannel.java | 2 +- .../net/unix/AFSomeSocketChannel.java | 58 +++++ .../newsclub/net/unix/FileDescriptorCast.java | 204 ++++++++++++++---- .../newsclub/net/unix/NativeUnixSocket.java | 9 + .../src/main/c/filedescriptors.c | 27 +++ .../org_newsclub_net_unix_NativeUnixSocket.h | 8 + 8 files changed, 269 insertions(+), 43 deletions(-) create mode 100644 junixsocket-common/src/main/java/org/newsclub/net/unix/AFSomeSocketChannel.java diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFDatagramChannel.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFDatagramChannel.java index 087c2ced9..18796d71b 100644 --- a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFDatagramChannel.java +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFDatagramChannel.java @@ -42,7 +42,7 @@ * @param The supported address type. */ public abstract class AFDatagramChannel extends DatagramChannel - implements AFSomeSocket, AFSocketExtensions { + implements AFSomeSocket, AFSocketExtensions, AFSomeSocketChannel { private final AFDatagramSocket afSocket; /** diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFServerSocketChannel.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFServerSocketChannel.java index bb97a96ec..be79ac413 100644 --- a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFServerSocketChannel.java +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFServerSocketChannel.java @@ -39,7 +39,7 @@ * @author Christian Kohlschütter */ public abstract class AFServerSocketChannel extends ServerSocketChannel - implements FileDescriptorAccess { + implements FileDescriptorAccess, AFSomeSocketChannel { private final @NonNull AFServerSocket afSocket; /** diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSocketChannel.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSocketChannel.java index e825e11ff..ce475a143 100644 --- a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSocketChannel.java +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSocketChannel.java @@ -39,7 +39,7 @@ * @author Christian Kohlschütter */ public abstract class AFSocketChannel extends SocketChannel implements - AFSomeSocket, AFSocketExtensions { + AFSomeSocket, AFSocketExtensions, AFSomeSocketChannel { private final @NonNull AFSocket afSocket; private final AtomicBoolean connectPending = new AtomicBoolean(false); diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSomeSocketChannel.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSomeSocketChannel.java new file mode 100644 index 000000000..99bbf3afa --- /dev/null +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSomeSocketChannel.java @@ -0,0 +1,58 @@ +/* + * junixsocket + * + * Copyright 2009-2023 Christian Kohlschütter + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.newsclub.net.unix; + +import java.io.Closeable; +import java.io.IOException; +import java.nio.channels.DatagramChannel; +import java.nio.channels.SelectableChannel; +import java.nio.channels.ServerSocketChannel; +import java.nio.channels.SocketChannel; + +/** + * Marker interface that combines junixsocket-based {@link SocketChannel}s, {@link DatagramChannel}s + * or {@link ServerSocketChannel}s. + * + * @author Christian Kohlschütter + * @see AFSocketChannel + * @see AFServerSocketChannel + * @see AFDatagramChannel + */ +public interface AFSomeSocketChannel extends Closeable, FileDescriptorAccess { + /** + * Checks if the channel is configured blocking. The result may be cached, and therefore not + * invoke native code to check if the underlying socket is actually configured that way. + * + * @return {@code true} if blocking. + */ + boolean isBlocking(); + + /** + * Adjusts this channel's blocking mode. + * + *

+ * If the given blocking mode is different from the currently cached blocking mode then this + * method native code to change it. + *

+ * + * @param block {@code true} if blocking is desired. + * @return This channel. + * @throws IOException on error. + */ + SelectableChannel configureBlocking(boolean block) throws IOException; +} diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/FileDescriptorCast.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/FileDescriptorCast.java index daaac2cf4..c893b4d47 100644 --- a/junixsocket-common/src/main/java/org/newsclub/net/unix/FileDescriptorCast.java +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/FileDescriptorCast.java @@ -24,10 +24,15 @@ import java.io.InputStream; import java.lang.ProcessBuilder.Redirect; import java.net.DatagramSocket; +import java.net.ServerSocket; import java.net.Socket; +import java.nio.channels.DatagramChannel; import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; +import java.nio.channels.ServerSocketChannel; +import java.nio.channels.SocketChannel; import java.nio.channels.WritableByteChannel; +import java.nio.channels.spi.AbstractSelectableChannel; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -66,6 +71,11 @@ * that do not encode this information directly (such as {@link AFUNIXSocketAddress} and * {@link AFTIPCSocketAddress}). *

+ *

+ * Also note that the "blocking" state of a socket may be forcibly changed to "blocking" when + * performing the cast, especially when casting to {@link Socket}, {@link DatagramSocket} or + * {@link ServerSocket} and any of their subclasses where "blocking" is the expected state. + *

* * @author Christian Kohlschütter */ @@ -225,21 +235,24 @@ static void registerCastingProviders( protected void addProviders() { addProviders(GLOBAL_PROVIDERS); - final CastingProvider> cpSocket = (fdc, desiredType) -> AFSocket.newInstance( - config.socketConstructor(), (AFSocketFactory) null, fdc.getFileDescriptor(), - fdc.localPort, fdc.remotePort); - final CastingProvider> cpServerSocket = (fdc, - desiredType) -> AFServerSocket.newInstance(config.serverSocketConstructor(), fdc - .getFileDescriptor(), fdc.localPort, fdc.remotePort); + final CastingProviderSocketOrChannel> cpSocketOrChannel = (fdc, desiredType, + isChannel) -> reconfigure(isChannel, AFSocket.newInstance(config.socketConstructor(), + (AFSocketFactory) null, fdc.getFileDescriptor(), fdc.localPort, fdc.remotePort)); + final CastingProviderSocketOrChannel> cpServerSocketOrChannel = (fdc, + desiredType, isChannel) -> reconfigure(isChannel, AFServerSocket.newInstance(config + .serverSocketConstructor(), fdc.getFileDescriptor(), fdc.localPort, + fdc.remotePort)); registerGenericSocketProviders(); - addProvider(socketClass, cpSocket); - addProvider(config.serverSocketClass(), cpServerSocket); - addProvider(config.socketChannelClass(), (fdc, desiredType) -> cpSocket.provideAs(fdc, - AFSocket.class).getChannel()); - addProvider(config.serverSocketChannelClass(), (fdc, desiredType) -> cpServerSocket - .provideAs(fdc, AFServerSocket.class).getChannel()); + addProvider(socketClass, (fdc, desiredType) -> cpSocketOrChannel.provideAs(fdc, desiredType, + false)); + addProvider(config.serverSocketClass(), (fdc, desiredType) -> cpServerSocketOrChannel + .provideAs(fdc, desiredType, false)); + addProvider(config.socketChannelClass(), (fdc, desiredType) -> cpSocketOrChannel.provideAs( + fdc, AFSocket.class, true).getChannel()); + addProvider(config.serverSocketChannelClass(), (fdc, desiredType) -> cpServerSocketOrChannel + .provideAs(fdc, AFServerSocket.class, true).getChannel()); } }); @@ -250,15 +263,17 @@ protected void addProviders() { protected void addProviders() { addProviders(GLOBAL_PROVIDERS); - final CastingProvider> cpDatagramSocket = (fdc, - desiredType) -> AFDatagramSocket.newInstance(config.datagramSocketConstructor(), fdc - .getFileDescriptor(), fdc.localPort, fdc.remotePort); + final CastingProviderSocketOrChannel> cpDatagramSocketOrChannel = (fdc, + desiredType, isChannel) -> reconfigure(isChannel, AFDatagramSocket.newInstance(config + .datagramSocketConstructor(), fdc.getFileDescriptor(), fdc.localPort, + fdc.remotePort)); registerGenericDatagramSocketProviders(); - addProvider(datagramSocketClass, cpDatagramSocket); - addProvider(config.datagramChannelClass(), (fdc, desiredType) -> cpDatagramSocket.provideAs( - fdc, AFDatagramSocket.class).getChannel()); + addProvider(datagramSocketClass, (fdc, desiredType) -> cpDatagramSocketOrChannel.provideAs( + fdc, desiredType, false)); + addProvider(config.datagramChannelClass(), (fdc, desiredType) -> cpDatagramSocketOrChannel + .provideAs(fdc, AFDatagramSocket.class, true).getChannel()); } }); } @@ -275,31 +290,35 @@ protected CastingProviderMap() { } protected void registerGenericSocketProviders() { - final CastingProvider> cpSocketGeneric = (fdc, - desiredType) -> AFSocket.newInstance(AFGenericSocket::new, - (AFSocketFactory) null, fdc.getFileDescriptor(), - fdc.localPort, fdc.remotePort); - final CastingProvider> cpServerSocketGeneric = (fdc, - desiredType) -> AFServerSocket.newInstance(AFGenericServerSocket::new, fdc - .getFileDescriptor(), fdc.localPort, fdc.remotePort); - - addProvider(AFGenericSocket.class, cpSocketGeneric); - addProvider(AFGenericServerSocket.class, cpServerSocketGeneric); - addProvider(AFGenericSocketChannel.class, (fdc, desiredType) -> cpSocketGeneric.provideAs(fdc, - AFSocket.class).getChannel()); - addProvider(AFGenericServerSocketChannel.class, (fdc, desiredType) -> cpServerSocketGeneric - .provideAs(fdc, AFServerSocket.class).getChannel()); + final CastingProviderSocketOrChannel> cpSocketOrChannelGeneric = + (fdc, desiredType, isChannel) -> reconfigure(isChannel, AFSocket.newInstance( + AFGenericSocket::new, (AFSocketFactory) null, fdc + .getFileDescriptor(), fdc.localPort, fdc.remotePort)); + final CastingProviderSocketOrChannel> cpServerSocketOrChannelGeneric = + (fdc, desiredType, isChannel) -> reconfigure(isChannel, AFServerSocket.newInstance( + AFGenericServerSocket::new, fdc.getFileDescriptor(), fdc.localPort, fdc.remotePort)); + + addProvider(AFGenericSocket.class, (fdc, desiredType) -> cpSocketOrChannelGeneric.provideAs( + fdc, desiredType, false)); + addProvider(AFGenericServerSocket.class, (fdc, desiredType) -> cpServerSocketOrChannelGeneric + .provideAs(fdc, desiredType, false)); + addProvider(AFGenericSocketChannel.class, (fdc, desiredType) -> cpSocketOrChannelGeneric + .provideAs(fdc, AFSocket.class, true).getChannel()); + addProvider(AFGenericServerSocketChannel.class, (fdc, + desiredType) -> cpServerSocketOrChannelGeneric.provideAs(fdc, AFServerSocket.class, true) + .getChannel()); } protected void registerGenericDatagramSocketProviders() { - final CastingProvider> cpDatagramSocketGeneric = ( - fdc, desiredType) -> AFDatagramSocket.newInstance(AFGenericDatagramSocket::new, fdc - .getFileDescriptor(), fdc.localPort, fdc.remotePort); - - addProvider(AFDatagramSocket.class, cpDatagramSocketGeneric); - addProvider(AFDatagramChannel.class, (fdc, desiredType) -> cpDatagramSocketGeneric.provideAs( - fdc, AFDatagramSocket.class).getChannel()); - + final CastingProviderSocketOrChannel> cpDatagramSocketOrChannelGeneric = + (fdc, desiredType, isChannel) -> reconfigure(isChannel, AFDatagramSocket.newInstance( + AFGenericDatagramSocket::new, fdc.getFileDescriptor(), fdc.localPort, + fdc.remotePort)); + + addProvider(AFDatagramSocket.class, (fdc, desiredType) -> cpDatagramSocketOrChannelGeneric + .provideAs(fdc, desiredType, false)); + addProvider(AFDatagramChannel.class, (fdc, desiredType) -> cpDatagramSocketOrChannelGeneric + .provideAs(fdc, AFDatagramSocket.class, true).getChannel()); } protected abstract void addProviders(); @@ -340,6 +359,12 @@ private interface CastingProvider { T provideAs(FileDescriptorCast fdc, Class desiredType) throws IOException; } + @FunctionalInterface + private interface CastingProviderSocketOrChannel { + T provideAs(FileDescriptorCast fdc, Class desiredType, boolean isChannel) + throws IOException; + } + /** * Creates a {@link FileDescriptorCast} using the given file descriptor. * @@ -555,4 +580,103 @@ protected void addProviders() { } }); } + + @SuppressWarnings("null") + private static > S reconfigure(boolean isChannel, S socket) + throws IOException { + reconfigure(isChannel, socket.getChannel()); + return socket; + } + + @SuppressWarnings("null") + private static > S reconfigure(boolean isChannel, S socket) + throws IOException { + reconfigure(isChannel, socket.getChannel()); + return socket; + } + + @SuppressWarnings("null") + private static > S reconfigure(boolean isChannel, S socket) + throws IOException { + reconfigure(isChannel, socket.getChannel()); + return socket; + } + + /** + * Reconfigures the Java-side of the socket/socket channel such that its state is compatible with + * the native socket's state. This is necessary to properly configure blocking/non-blocking state, + * as that is cached on the Java side. + *

+ * If {@code isChannel} is false, then we want to cast to a {@link Socket}, {@link DatagramSocket} + * or {@link ServerSocket}, which means blocking I/O is desired. If the underlying native socket + * is configured non-blocking, we need to reset the state to "blocking" accordingly. + *

+ * If {@code isChannel} is true, then we want to cast to a {@link SocketChannel}, + * {@link DatagramChannel} or {@link ServerSocketChannel}, in which case the blocking state should + * be preserved, if possible. It is then up to the user to check blocking state via + * {@link AbstractSelectableChannel#isBlocking()} prior to using the socket. + *

+ * Note that on Windows, it may be impossible to query the blocking state from an external socket, + * so the state is always forcibly set to "blocking". + * + * @param The type. + * @param isChannel The desired cast type (socket=set to blocking, or channel=preserve state). + * @param socketChannel The channel. + * @throws IOException on error. + */ + private static <@NonNull S extends AFSomeSocketChannel> void reconfigure(boolean isChannel, + S socketChannel) throws IOException { + if (isChannel) { + reconfigureKeepBlockingState(socketChannel); + } else { + reconfigureSetBlocking(socketChannel); + } + } + + private static <@NonNull S extends AFSomeSocketChannel> void reconfigureKeepBlockingState( + S socketChannel) throws IOException { + int result = NativeUnixSocket.checkBlocking(socketChannel.getFileDescriptor()); + + boolean blocking; + switch (result) { + case 0: + blocking = false; + break; + case 1: + blocking = true; + break; + case 2: + // need to reconfigure/forcibly override any cached result -> set to blocking by default + socketChannel.configureBlocking(false); + socketChannel.configureBlocking(true); + return; + default: + throw new OperationNotSupportedSocketException("Invalid blocking state"); + } + + socketChannel.configureBlocking(blocking); + } + + private static <@NonNull S extends AFSomeSocketChannel> void reconfigureSetBlocking( + S socketChannel) throws IOException { + int result = NativeUnixSocket.checkBlocking(socketChannel.getFileDescriptor()); + + switch (result) { + case 0: + // see below + break; + case 1: + // already blocking, nothing to do + return; + case 2: + // need to reconfigure/forcibly override any cached result -> set to blocking by default + // see below + break; + default: + throw new OperationNotSupportedSocketException("Invalid blocking state"); + } + + socketChannel.configureBlocking(false); + socketChannel.configureBlocking(true); + } } diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/NativeUnixSocket.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/NativeUnixSocket.java index c17e73834..0fc427997 100644 --- a/junixsocket-common/src/main/java/org/newsclub/net/unix/NativeUnixSocket.java +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/NativeUnixSocket.java @@ -298,6 +298,15 @@ static native boolean initPipe(FileDescriptor source, FileDescriptor sink, boole static native void configureBlocking(FileDescriptor fd, boolean blocking) throws IOException; + /** + * Checks if the given file descriptor describes a blocking socket. + * + * @param fd The file descriptor to check + * @return 0 = non-blocking, 1 = blocking, 2 = indeterminate (needs reconfiguration) + * @throws IOException on error. + */ + static native int checkBlocking(FileDescriptor fd) throws IOException; + static native void socketPair(int domain, int type, FileDescriptor fd, FileDescriptor fd2); static native Redirect initRedirect(FileDescriptor fd); diff --git a/junixsocket-native/src/main/c/filedescriptors.c b/junixsocket-native/src/main/c/filedescriptors.c index 0e3df008d..0ecfca2f9 100644 --- a/junixsocket-native/src/main/c/filedescriptors.c +++ b/junixsocket-native/src/main/c/filedescriptors.c @@ -338,6 +338,33 @@ JNIEXPORT void JNICALL Java_org_newsclub_net_unix_NativeUnixSocket_shutdown } } +/* + * Class: org_newsclub_net_unix_NativeUnixSocket + * Method: checkBlocking + * Signature: (Ljava/io/FileDescriptor;)I + */ +JNIEXPORT jint JNICALL Java_org_newsclub_net_unix_NativeUnixSocket_checkBlocking + (JNIEnv *env, jclass clazz CK_UNUSED, jobject fd) { + int handle = _getFD(env, fd); +#if defined(_WIN32) + CK_ARGUMENT_POTENTIALLY_UNUSED(handle); + // Windows doesn't provide current API to check for blocking state + return 2; // "indeterminate; needs re-configure" +#else + int flags = fcntl(handle, F_GETFL); + if(flags == -1) { + _throwErrnumException(env, socket_errno, NULL); + return -1; + } + + if((flags & O_NONBLOCK) != 0) { + return 0; // "non-blocking" + } else { + return 1; // "blocking" + } +#endif +} + /* * Class: org_newsclub_net_unix_NativeUnixSocket * Method: configureBlocking diff --git a/junixsocket-native/src/main/c/org_newsclub_net_unix_NativeUnixSocket.h b/junixsocket-native/src/main/c/org_newsclub_net_unix_NativeUnixSocket.h index 507ae4ae8..54fcf19d5 100644 --- a/junixsocket-native/src/main/c/org_newsclub_net_unix_NativeUnixSocket.h +++ b/junixsocket-native/src/main/c/org_newsclub_net_unix_NativeUnixSocket.h @@ -381,6 +381,14 @@ JNIEXPORT jint JNICALL Java_org_newsclub_net_unix_NativeUnixSocket_poll JNIEXPORT void JNICALL Java_org_newsclub_net_unix_NativeUnixSocket_configureBlocking (JNIEnv *, jclass, jobject, jboolean); +/* + * Class: org_newsclub_net_unix_NativeUnixSocket + * Method: checkBlocking + * Signature: (Ljava/io/FileDescriptor;)I + */ +JNIEXPORT jint JNICALL Java_org_newsclub_net_unix_NativeUnixSocket_checkBlocking + (JNIEnv *, jclass, jobject); + /* * Class: org_newsclub_net_unix_NativeUnixSocket * Method: socketPair