Skip to content

Commit 1866161

Browse files
crazyrokrCopilot
andauthored
Propagate Connection Closure to Client (JavaSaBr#64)
--------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 254fe77 commit 1866161

9 files changed

Lines changed: 261 additions & 7 deletions

File tree

rlib-common/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@ plugins {
66
dependencies {
77
api projects.rlibLoggerApi
88
api projects.rlibFunctions
9+
testFixturesImplementation libs.lombok
910
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package javasabr.rlib.common.util;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.util.concurrent.TimeUnit;
6+
import java.util.concurrent.atomic.AtomicBoolean;
7+
import org.junit.jupiter.api.Test;
8+
9+
/**
10+
* Tests of {@link AwaitUtils} methods.
11+
*
12+
* @author crazyrokr
13+
*/
14+
public class AwaitUtilsTest {
15+
16+
@Test
17+
void shouldAwaitCondition() throws InterruptedException {
18+
// given
19+
var condition = new AtomicBoolean(false);
20+
var thread = new Thread(() -> {
21+
try {
22+
Thread.sleep(100);
23+
condition.set(true);
24+
} catch (InterruptedException e) {
25+
// ignore
26+
}
27+
});
28+
29+
// when
30+
thread.start();
31+
boolean result = AwaitUtils.await(500, TimeUnit.MILLISECONDS, condition::get);
32+
33+
// then
34+
assertThat(result).isTrue();
35+
}
36+
37+
@Test
38+
void shouldTimeoutIfConditionNotMet() throws InterruptedException {
39+
// given
40+
var condition = new AtomicBoolean(false);
41+
42+
// when
43+
boolean result = AwaitUtils.await(100, TimeUnit.MILLISECONDS, condition::get);
44+
45+
// then
46+
assertThat(result).isFalse();
47+
}
48+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package javasabr.rlib.common.util;
2+
3+
import java.util.concurrent.TimeUnit;
4+
import java.util.function.Supplier;
5+
import lombok.experimental.UtilityClass;
6+
7+
/**
8+
* The utility class to await some conditions.
9+
*
10+
* @author crazyrokr
11+
*/
12+
@UtilityClass
13+
public final class AwaitUtils {
14+
15+
/**
16+
* Await for the condition during the amount of time units.
17+
*
18+
* @param amount the amount of time units.
19+
* @param unit the time unit.
20+
* @param condition the condition.
21+
* @return true if the condition was met.
22+
* @throws InterruptedException if the current thread was interrupted.
23+
*/
24+
public static boolean await(long amount, TimeUnit unit, Supplier<Boolean> condition) throws InterruptedException {
25+
if (condition.get()) {
26+
return true;
27+
}
28+
var timeoutMillis = unit.toMillis(amount);
29+
var endTime = System.currentTimeMillis() + timeoutMillis;
30+
while (System.currentTimeMillis() < endTime) {
31+
if (condition.get()) {
32+
return true;
33+
}
34+
Thread.sleep(Math.clamp(endTime - System.currentTimeMillis(), 1, 10));
35+
}
36+
return condition.get();
37+
}
38+
}

rlib-network/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,5 @@ dependencies {
1111
api libs.project.reactor.core
1212
testRuntimeOnly projects.rlibLoggerImpl
1313
loadTestRuntimeOnly projects.rlibLoggerImpl
14+
testImplementation testFixtures(projects.rlibCommon)
1415
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package javasabr.rlib.network.exception;
2+
3+
/**
4+
* Thrown when a network connection has been closed
5+
*
6+
* @since 10.0.0
7+
*/
8+
public class ConnectionClosedException extends NetworkException {
9+
10+
/**
11+
* Creates a new exception for a closed connection
12+
*
13+
* @param remoteAddress the remote address
14+
*/
15+
public ConnectionClosedException(String remoteAddress) {
16+
super("Connection closed: %s".formatted(remoteAddress));
17+
}
18+
19+
/**
20+
* Creates a new exception for a closed connection with a cause
21+
*
22+
* @param remoteAddress the remote address
23+
* @param cause the cause
24+
*/
25+
public ConnectionClosedException(String remoteAddress, Throwable cause) {
26+
super("Connection closed: %s".formatted(remoteAddress), cause);
27+
}
28+
}

rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,23 @@
44

55
import java.nio.channels.AsynchronousChannel;
66
import java.nio.channels.AsynchronousSocketChannel;
7+
import java.util.Collection;
78
import java.util.Deque;
89
import java.util.concurrent.CompletableFuture;
910
import java.util.concurrent.atomic.AtomicBoolean;
1011
import java.util.concurrent.locks.StampedLock;
1112
import java.util.function.BiConsumer;
13+
import javasabr.rlib.collections.array.Array;
1214
import javasabr.rlib.collections.array.ArrayFactory;
15+
import javasabr.rlib.collections.array.LockableArray;
1316
import javasabr.rlib.collections.array.MutableArray;
1417
import javasabr.rlib.collections.deque.DequeFactory;
18+
import javasabr.rlib.collections.operation.LockableOperations;
1519
import javasabr.rlib.network.BufferAllocator;
1620
import javasabr.rlib.network.Connection;
1721
import javasabr.rlib.network.Network;
1822
import javasabr.rlib.network.UnsafeConnection;
23+
import javasabr.rlib.network.exception.ConnectionClosedException;
1924
import javasabr.rlib.network.packet.NetworkPacketReader;
2025
import javasabr.rlib.network.packet.NetworkPacketWriter;
2126
import javasabr.rlib.network.packet.ReadableNetworkPacket;
@@ -64,6 +69,8 @@ public WritablePacketWithFeedback(CompletableFuture<Boolean> attachment, Writabl
6469

6570
final MutableArray<BiConsumer<C, ? super ReadableNetworkPacket<C>>> validPacketSubscribers;
6671
final MutableArray<BiConsumer<C, ? super ReadableNetworkPacket<C>>> invalidPacketSubscribers;
72+
final LockableArray<FluxSink<?>> activeSinks;
73+
final LockableOperations<LockableArray<FluxSink<?>>> activeSinksOperations;
6774

6875
final int maxPacketsByRead;
6976

@@ -84,6 +91,8 @@ public AbstractConnection(
8491
this.closed = new AtomicBoolean(false);
8592
this.validPacketSubscribers = ArrayFactory.copyOnModifyArray(BiConsumer.class);
8693
this.invalidPacketSubscribers = ArrayFactory.copyOnModifyArray(BiConsumer.class);
94+
this.activeSinks = ArrayFactory.stampedLockBasedArray(FluxSink.class);
95+
this.activeSinksOperations = activeSinks.operations();
8796
this.remoteAddress = String.valueOf(NetworkUtils.getRemoteAddress(channel));
8897
}
8998

@@ -134,10 +143,12 @@ protected void registerFluxOnReceivedEvents(
134143

135144
validPacketSubscribers.add(validListener);
136145
invalidPacketSubscribers.add(invalidListener);
146+
activeSinksOperations.inWriteLock(sink, Collection::add);
137147

138148
sink.onDispose(() -> {
139149
validPacketSubscribers.remove(validListener);
140-
validPacketSubscribers.remove(invalidListener);
150+
invalidPacketSubscribers.remove(invalidListener);
151+
activeSinksOperations.inWriteLock(sink, Collection::remove);
141152
});
142153

143154
network.inNetworkThread(() -> packetReader().startRead());
@@ -146,14 +157,22 @@ protected void registerFluxOnReceivedEvents(
146157
protected void registerFluxOnReceivedValidPackets(FluxSink<? super ReadableNetworkPacket<C>> sink) {
147158
BiConsumer<C, ReadableNetworkPacket<C>> listener = (connection, packet) -> sink.next(packet);
148159
validPacketSubscribers.add(listener);
149-
sink.onDispose(() -> validPacketSubscribers.remove(listener));
160+
activeSinksOperations.inWriteLock(sink, Collection::add);
161+
sink.onDispose(() -> {
162+
validPacketSubscribers.remove(listener);
163+
activeSinksOperations.inWriteLock(sink, Collection::remove);
164+
});
150165
network.inNetworkThread(() -> packetReader().startRead());
151166
}
152167

153168
protected void registerFluxOnReceivedInvalidPackets(FluxSink<? super ReadableNetworkPacket<C>> sink) {
154169
BiConsumer<C, ReadableNetworkPacket<C>> listener = (connection, packet) -> sink.next(packet);
155170
invalidPacketSubscribers.add(listener);
156-
sink.onDispose(() -> invalidPacketSubscribers.remove(listener));
171+
activeSinksOperations.inWriteLock(sink, Collection::add);
172+
sink.onDispose(() -> {
173+
invalidPacketSubscribers.remove(listener);
174+
activeSinksOperations.inWriteLock(sink, Collection::remove);
175+
});
157176
network.inNetworkThread(() -> packetReader().startRead());
158177
}
159178

@@ -184,6 +203,27 @@ protected void doClose() {
184203
clearWaitPackets();
185204
packetReader().close();
186205
packetWriter().close();
206+
notifyActiveSinks();
207+
}
208+
209+
protected void notifyActiveSinks() {
210+
Boolean noActiveSinks = activeSinksOperations.getInReadLock(Array::isEmpty);
211+
if (noActiveSinks) {
212+
return;
213+
}
214+
notifySinksWithError(new ConnectionClosedException(remoteAddress));
215+
activeSinksOperations.inWriteLock(Collection::clear);
216+
}
217+
218+
protected void notifySinksWithError(Throwable error) {
219+
Array<FluxSink<?>> localActiveSinks = activeSinksOperations.getInReadLock(Array::copyOf);
220+
for (FluxSink<?> sink : localActiveSinks) {
221+
try {
222+
sink.error(error);
223+
} catch (RuntimeException e) {
224+
log.error(e.getMessage(), "Failed to notify sink of connection closure: "::formatted);
225+
}
226+
}
187227
}
188228

189229
/**

rlib-network/src/main/java/javasabr/rlib/network/packet/impl/AbstractNetworkPacketReader.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,10 +461,14 @@ protected void handleFailedReceiving(Throwable exception, ByteBuffer readingBuff
461461
retryReadLater();
462462
}
463463
}
464-
case AsynchronousCloseException ex ->
465-
log.info(remoteAddress(), "[%s] Connection was closed"::formatted);
466-
case ClosedChannelException ex ->
467-
log.info(remoteAddress(), "[%s] Connection was closed"::formatted);
464+
case AsynchronousCloseException ex -> {
465+
log.info(remoteAddress(), "[%s] Connection was closed"::formatted);
466+
connection.close();
467+
}
468+
case ClosedChannelException ex -> {
469+
log.info(remoteAddress(), "[%s] Connection was closed"::formatted);
470+
connection.close();
471+
}
468472
default -> {
469473
log.error(exception);
470474
connection.close();

rlib-network/src/main/java/javasabr/rlib/network/packet/impl/AbstractSslNetworkPacketReader.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ protected AbstractSslNetworkPacketReader(
7676
protected void handleReceivedData(int receivedBytes, ByteBuffer readingBuffer) {
7777
if (receivedBytes == -1) {
7878
doHandshake(sslNetworkBuffer(), -1);
79+
handleEmptyReadFromChannel();
7980
return;
8081
}
8182
super.handleReceivedData(receivedBytes, readingBuffer);
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package javasabr.rlib.network;
2+
3+
import static javasabr.rlib.network.util.NetworkUtils.createAllTrustedClientSslContext;
4+
import static javasabr.rlib.network.util.NetworkUtils.createSslContext;
5+
import static org.assertj.core.api.Assertions.assertThat;
6+
7+
import java.net.InetSocketAddress;
8+
import java.util.concurrent.CountDownLatch;
9+
import java.util.concurrent.TimeUnit;
10+
import javasabr.rlib.common.util.AwaitUtils;
11+
import javasabr.rlib.network.exception.ConnectionClosedException;
12+
import javasabr.rlib.network.impl.AbstractConnection;
13+
import javasabr.rlib.network.impl.DefaultConnection;
14+
import javasabr.rlib.network.packet.impl.DefaultReadableNetworkPacket;
15+
import javasabr.rlib.network.packet.impl.StringWritableNetworkPacket;
16+
import javasabr.rlib.network.packet.registry.ReadableNetworkPacketRegistry;
17+
import lombok.SneakyThrows;
18+
import org.junit.jupiter.api.Test;
19+
20+
/**
21+
* Checking that the connections are closed correctly
22+
*
23+
* @author crazyrokr
24+
*/
25+
public class ConnectionCloseTest extends BaseNetworkTest {
26+
27+
@Test
28+
void shouldPropagateConnectionCloseToClient() throws InterruptedException {
29+
// given
30+
var packetRegistry = ReadableNetworkPacketRegistry.of(
31+
DefaultReadableNetworkPacket.class,
32+
DefaultConnection.class,
33+
DefaultNetworkTest.ServerPackets.RequestEchoMessage.class,
34+
DefaultNetworkTest.ServerPackets.RequestServerTime.class);
35+
var serverNetwork = NetworkFactory.defaultServerNetwork(packetRegistry);
36+
InetSocketAddress serverAddress = serverNetwork.start();
37+
serverNetwork.onAccept(AbstractConnection::close);
38+
var clientNetwork = NetworkFactory.defaultClientNetwork(packetRegistry);
39+
CountDownLatch closeLatch = new CountDownLatch(1);
40+
41+
// when
42+
try {
43+
clientNetwork
44+
.connectReactive(serverAddress)
45+
.flatMapMany(AbstractConnection::receivedEvents)
46+
.doOnError(e -> {
47+
if (e instanceof ConnectionClosedException) {
48+
closeLatch.countDown();
49+
}
50+
})
51+
.subscribe();
52+
53+
// then
54+
assertThat(closeLatch.await(5000, TimeUnit.MILLISECONDS))
55+
.as("Client should be notified that connection is closed")
56+
.isTrue();
57+
} finally {
58+
// cleanup
59+
clientNetwork.shutdown();
60+
serverNetwork.shutdown();
61+
}
62+
}
63+
64+
@Test
65+
@SneakyThrows
66+
void shouldCloseServerConnectionWhenClientClosesTcpChannelAbruptly() {
67+
// given
68+
try (var keystoreFile = ConnectionCloseTest.class.getResourceAsStream("/ssl/rlib_test_cert.p12");
69+
var testNetwork = buildStringSSLNetwork(
70+
createSslContext(keystoreFile, "test"),
71+
createAllTrustedClientSslContext())) {
72+
var serverConnection = testNetwork.serverToClient;
73+
var clientConnection = testNetwork.clientToServer;
74+
CountDownLatch dataReceivedLatch = new CountDownLatch(1);
75+
serverConnection.onReceiveValidPacket((conn, packet) -> dataReceivedLatch.countDown());
76+
clientConnection.sendInBackground(new StringWritableNetworkPacket<>("handshake"));
77+
assertThat(dataReceivedLatch.await(5, TimeUnit.SECONDS))
78+
.as("Client connection should be closed prior server side verification")
79+
.isTrue();
80+
81+
// when
82+
clientConnection.channel().close();
83+
assertThat(AwaitUtils.await(5, TimeUnit.SECONDS, clientConnection::closed))
84+
.as("Client connection should be closed prior server side verification")
85+
.isTrue();
86+
87+
// then
88+
assertThat(AwaitUtils.await(5, TimeUnit.SECONDS, serverConnection::closed))
89+
.as("Server connection should be closed after receiving EOF from abruptly closed client channel")
90+
.isTrue();
91+
}
92+
}
93+
}

0 commit comments

Comments
 (0)