-
Notifications
You must be signed in to change notification settings - Fork 6
Propagate Connection Closure to Client #64
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
Changes from all commits
1bab899
e4a1c33
6b19a97
eaf9276
1a0244c
e517502
8b7509c
35939ce
514b2e5
78dedb9
e7d39f7
13db757
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package javasabr.rlib.common.util; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| /** | ||
| * Tests of {@link AwaitUtils} methods. | ||
| * | ||
| * @author crazyrokr | ||
| */ | ||
| public class AwaitUtilsTest { | ||
|
|
||
| @Test | ||
| void shouldAwaitCondition() throws InterruptedException { | ||
| // given | ||
| var condition = new AtomicBoolean(false); | ||
| var thread = new Thread(() -> { | ||
| try { | ||
| Thread.sleep(100); | ||
| condition.set(true); | ||
| } catch (InterruptedException e) { | ||
| // ignore | ||
| } | ||
| }); | ||
|
|
||
| // when | ||
| thread.start(); | ||
| boolean result = AwaitUtils.await(500, TimeUnit.MILLISECONDS, condition::get); | ||
|
|
||
| // then | ||
| assertThat(result).isTrue(); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldTimeoutIfConditionNotMet() throws InterruptedException { | ||
| // given | ||
| var condition = new AtomicBoolean(false); | ||
|
|
||
| // when | ||
| boolean result = AwaitUtils.await(100, TimeUnit.MILLISECONDS, condition::get); | ||
|
|
||
| // then | ||
| assertThat(result).isFalse(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| package javasabr.rlib.common.util; | ||
|
|
||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.function.Supplier; | ||
| import lombok.experimental.UtilityClass; | ||
|
|
||
| /** | ||
| * The utility class to await some conditions. | ||
| * | ||
| * @author crazyrokr | ||
| */ | ||
| @UtilityClass | ||
| public final class AwaitUtils { | ||
|
|
||
| /** | ||
| * Await for the condition during the amount of time units. | ||
| * | ||
| * @param amount the amount of time units. | ||
| * @param unit the time unit. | ||
| * @param condition the condition. | ||
| * @return true if the condition was met. | ||
| * @throws InterruptedException if the current thread was interrupted. | ||
| */ | ||
| public static boolean await(long amount, TimeUnit unit, Supplier<Boolean> condition) throws InterruptedException { | ||
| if (condition.get()) { | ||
| return true; | ||
| } | ||
| var timeoutMillis = unit.toMillis(amount); | ||
| var endTime = System.currentTimeMillis() + timeoutMillis; | ||
| while (System.currentTimeMillis() < endTime) { | ||
| if (condition.get()) { | ||
| return true; | ||
| } | ||
| Thread.sleep(Math.clamp(endTime - System.currentTimeMillis(), 1, 10)); | ||
| } | ||
| return condition.get(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package javasabr.rlib.network.exception; | ||
|
|
||
| /** | ||
| * Thrown when a network connection has been closed | ||
| * | ||
| * @since 10.0.0 | ||
| */ | ||
| public class ConnectionClosedException extends NetworkException { | ||
|
|
||
| /** | ||
| * Creates a new exception for a closed connection | ||
| * | ||
| * @param remoteAddress the remote address | ||
| */ | ||
| public ConnectionClosedException(String remoteAddress) { | ||
| super("Connection closed: %s".formatted(remoteAddress)); | ||
| } | ||
|
|
||
|
crazyrokr marked this conversation as resolved.
|
||
| /** | ||
| * Creates a new exception for a closed connection with a cause | ||
| * | ||
| * @param remoteAddress the remote address | ||
| * @param cause the cause | ||
| */ | ||
| public ConnectionClosedException(String remoteAddress, Throwable cause) { | ||
| super("Connection closed: %s".formatted(remoteAddress), cause); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,18 +4,23 @@ | |
|
|
||
| import java.nio.channels.AsynchronousChannel; | ||
| import java.nio.channels.AsynchronousSocketChannel; | ||
| import java.util.Collection; | ||
| import java.util.Deque; | ||
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import java.util.concurrent.locks.StampedLock; | ||
| import java.util.function.BiConsumer; | ||
| import javasabr.rlib.collections.array.Array; | ||
| import javasabr.rlib.collections.array.ArrayFactory; | ||
| import javasabr.rlib.collections.array.LockableArray; | ||
| import javasabr.rlib.collections.array.MutableArray; | ||
| import javasabr.rlib.collections.deque.DequeFactory; | ||
| import javasabr.rlib.collections.operation.LockableOperations; | ||
| import javasabr.rlib.network.BufferAllocator; | ||
| import javasabr.rlib.network.Connection; | ||
| import javasabr.rlib.network.Network; | ||
| import javasabr.rlib.network.UnsafeConnection; | ||
| import javasabr.rlib.network.exception.ConnectionClosedException; | ||
| import javasabr.rlib.network.packet.NetworkPacketReader; | ||
| import javasabr.rlib.network.packet.NetworkPacketWriter; | ||
| import javasabr.rlib.network.packet.ReadableNetworkPacket; | ||
|
|
@@ -64,6 +69,8 @@ public WritablePacketWithFeedback(CompletableFuture<Boolean> attachment, Writabl | |
|
|
||
| final MutableArray<BiConsumer<C, ? super ReadableNetworkPacket<C>>> validPacketSubscribers; | ||
| final MutableArray<BiConsumer<C, ? super ReadableNetworkPacket<C>>> invalidPacketSubscribers; | ||
| final LockableArray<FluxSink<?>> activeSinks; | ||
| final LockableOperations<LockableArray<FluxSink<?>>> activeSinksOperations; | ||
|
|
||
| final int maxPacketsByRead; | ||
|
|
||
|
|
@@ -84,6 +91,8 @@ public AbstractConnection( | |
| this.closed = new AtomicBoolean(false); | ||
| this.validPacketSubscribers = ArrayFactory.copyOnModifyArray(BiConsumer.class); | ||
| this.invalidPacketSubscribers = ArrayFactory.copyOnModifyArray(BiConsumer.class); | ||
| this.activeSinks = ArrayFactory.stampedLockBasedArray(FluxSink.class); | ||
| this.activeSinksOperations = activeSinks.operations(); | ||
| this.remoteAddress = String.valueOf(NetworkUtils.getRemoteAddress(channel)); | ||
| } | ||
|
|
||
|
|
@@ -134,10 +143,12 @@ protected void registerFluxOnReceivedEvents( | |
|
|
||
| validPacketSubscribers.add(validListener); | ||
| invalidPacketSubscribers.add(invalidListener); | ||
| activeSinksOperations.inWriteLock(sink, Collection::add); | ||
|
|
||
| sink.onDispose(() -> { | ||
| validPacketSubscribers.remove(validListener); | ||
| validPacketSubscribers.remove(invalidListener); | ||
| invalidPacketSubscribers.remove(invalidListener); | ||
| activeSinksOperations.inWriteLock(sink, Collection::remove); | ||
| }); | ||
|
|
||
| network.inNetworkThread(() -> packetReader().startRead()); | ||
|
|
@@ -146,14 +157,22 @@ protected void registerFluxOnReceivedEvents( | |
| protected void registerFluxOnReceivedValidPackets(FluxSink<? super ReadableNetworkPacket<C>> sink) { | ||
| BiConsumer<C, ReadableNetworkPacket<C>> listener = (connection, packet) -> sink.next(packet); | ||
| validPacketSubscribers.add(listener); | ||
| sink.onDispose(() -> validPacketSubscribers.remove(listener)); | ||
| activeSinksOperations.inWriteLock(sink, Collection::add); | ||
| sink.onDispose(() -> { | ||
| validPacketSubscribers.remove(listener); | ||
| activeSinksOperations.inWriteLock(sink, Collection::remove); | ||
| }); | ||
| network.inNetworkThread(() -> packetReader().startRead()); | ||
| } | ||
|
|
||
| protected void registerFluxOnReceivedInvalidPackets(FluxSink<? super ReadableNetworkPacket<C>> sink) { | ||
| BiConsumer<C, ReadableNetworkPacket<C>> listener = (connection, packet) -> sink.next(packet); | ||
| invalidPacketSubscribers.add(listener); | ||
| sink.onDispose(() -> invalidPacketSubscribers.remove(listener)); | ||
| activeSinksOperations.inWriteLock(sink, Collection::add); | ||
| sink.onDispose(() -> { | ||
| invalidPacketSubscribers.remove(listener); | ||
| activeSinksOperations.inWriteLock(sink, Collection::remove); | ||
| }); | ||
| network.inNetworkThread(() -> packetReader().startRead()); | ||
| } | ||
|
|
||
|
|
@@ -184,6 +203,27 @@ protected void doClose() { | |
| clearWaitPackets(); | ||
| packetReader().close(); | ||
| packetWriter().close(); | ||
| notifyActiveSinks(); | ||
| } | ||
|
|
||
| protected void notifyActiveSinks() { | ||
| Boolean noActiveSinks = activeSinksOperations.getInReadLock(Array::isEmpty); | ||
| if (noActiveSinks) { | ||
| return; | ||
| } | ||
| notifySinksWithError(new ConnectionClosedException(remoteAddress)); | ||
| activeSinksOperations.inWriteLock(Collection::clear); | ||
| } | ||
|
|
||
| protected void notifySinksWithError(Throwable error) { | ||
| Array<FluxSink<?>> localActiveSinks = activeSinksOperations.getInReadLock(Array::copyOf); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you really want to allocate a full array for such reading?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so because |
||
| for (FluxSink<?> sink : localActiveSinks) { | ||
| try { | ||
| sink.error(error); | ||
| } catch (RuntimeException e) { | ||
| log.error(e.getMessage(), "Failed to notify sink of connection closure: "::formatted); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| package javasabr.rlib.network; | ||
|
|
||
| import static javasabr.rlib.network.util.NetworkUtils.createAllTrustedClientSslContext; | ||
| import static javasabr.rlib.network.util.NetworkUtils.createSslContext; | ||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.net.InetSocketAddress; | ||
| import java.util.concurrent.CountDownLatch; | ||
| import java.util.concurrent.TimeUnit; | ||
| import javasabr.rlib.common.util.AwaitUtils; | ||
| import javasabr.rlib.network.exception.ConnectionClosedException; | ||
| import javasabr.rlib.network.impl.AbstractConnection; | ||
| import javasabr.rlib.network.impl.DefaultConnection; | ||
| import javasabr.rlib.network.packet.impl.DefaultReadableNetworkPacket; | ||
| import javasabr.rlib.network.packet.impl.StringWritableNetworkPacket; | ||
| import javasabr.rlib.network.packet.registry.ReadableNetworkPacketRegistry; | ||
| import lombok.SneakyThrows; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| /** | ||
| * Checking that the connections are closed correctly | ||
| * | ||
| * @author crazyrokr | ||
| */ | ||
| public class ConnectionCloseTest extends BaseNetworkTest { | ||
|
|
||
| @Test | ||
| void shouldPropagateConnectionCloseToClient() throws InterruptedException { | ||
| // given | ||
| var packetRegistry = ReadableNetworkPacketRegistry.of( | ||
| DefaultReadableNetworkPacket.class, | ||
| DefaultConnection.class, | ||
| DefaultNetworkTest.ServerPackets.RequestEchoMessage.class, | ||
| DefaultNetworkTest.ServerPackets.RequestServerTime.class); | ||
| var serverNetwork = NetworkFactory.defaultServerNetwork(packetRegistry); | ||
| InetSocketAddress serverAddress = serverNetwork.start(); | ||
| serverNetwork.onAccept(AbstractConnection::close); | ||
| var clientNetwork = NetworkFactory.defaultClientNetwork(packetRegistry); | ||
| CountDownLatch closeLatch = new CountDownLatch(1); | ||
|
|
||
| // when | ||
| try { | ||
| clientNetwork | ||
| .connectReactive(serverAddress) | ||
| .flatMapMany(AbstractConnection::receivedEvents) | ||
| .doOnError(e -> { | ||
| if (e instanceof ConnectionClosedException) { | ||
| closeLatch.countDown(); | ||
| } | ||
| }) | ||
| .subscribe(); | ||
|
|
||
| // then | ||
| assertThat(closeLatch.await(5000, TimeUnit.MILLISECONDS)) | ||
| .as("Client should be notified that connection is closed") | ||
| .isTrue(); | ||
| } finally { | ||
| // cleanup | ||
| clientNetwork.shutdown(); | ||
| serverNetwork.shutdown(); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @SneakyThrows | ||
| void shouldCloseServerConnectionWhenClientClosesTcpChannelAbruptly() { | ||
| // given | ||
| try (var keystoreFile = ConnectionCloseTest.class.getResourceAsStream("/ssl/rlib_test_cert.p12"); | ||
| var testNetwork = buildStringSSLNetwork( | ||
| createSslContext(keystoreFile, "test"), | ||
| createAllTrustedClientSslContext())) { | ||
| var serverConnection = testNetwork.serverToClient; | ||
| var clientConnection = testNetwork.clientToServer; | ||
| CountDownLatch dataReceivedLatch = new CountDownLatch(1); | ||
| serverConnection.onReceiveValidPacket((conn, packet) -> dataReceivedLatch.countDown()); | ||
| clientConnection.sendInBackground(new StringWritableNetworkPacket<>("handshake")); | ||
| assertThat(dataReceivedLatch.await(5, TimeUnit.SECONDS)) | ||
| .as("Client connection should be closed prior server side verification") | ||
| .isTrue(); | ||
|
|
||
| // when | ||
| clientConnection.channel().close(); | ||
| assertThat(AwaitUtils.await(5, TimeUnit.SECONDS, clientConnection::closed)) | ||
| .as("Client connection should be closed prior server side verification") | ||
| .isTrue(); | ||
|
|
||
| // then | ||
| assertThat(AwaitUtils.await(5, TimeUnit.SECONDS, serverConnection::closed)) | ||
| .as("Server connection should be closed after receiving EOF from abruptly closed client channel") | ||
| .isTrue(); | ||
| } | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.