Skip to content

Commit 19b556f

Browse files
authored
Ensure CLOSE_FRAME can be produced when forceClose() is called (#360)
Motivation: Due a regression introduced in d7b6e36 we did not send the CLOSE_FRAME anymore in some cases Modifications: - Only set the connection to null after we are done with sending the CLOSE_FRAME - Combine reentrance state into one field and use bitwise ops to reduce memory overhead Result: Always produce the CLOSE_FRAME
1 parent c981833 commit 19b556f

File tree

1 file changed

+23
-17
lines changed

1 file changed

+23
-17
lines changed

codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,12 @@ public void operationComplete(ChannelFuture future) {
131131

132132
private boolean recvStreamPending;
133133
private boolean streamReadable;
134-
private boolean inRecv;
135-
private boolean inConnectionSend;
136-
private boolean inHandleWritableStreams;
134+
135+
private int reantranceGuard = 0;
136+
private static final int IN_RECV = 1 << 1;
137+
private static final int IN_CONNECTION_SEND = 1 << 2;
138+
private static final int IN_HANDLE_WRITABLE_STREAMS = 1 << 3;
139+
private static final int IN_FORCE_CLOSE = 1 << 3;
137140

138141
private static final int CLOSED = 0;
139142
private static final int OPEN = 1;
@@ -346,13 +349,13 @@ boolean markInFireChannelReadCompleteQueue() {
346349
}
347350

348351
void forceClose() {
349-
if (isConnDestroyed()) {
352+
if (isConnDestroyed() || (reantranceGuard & IN_FORCE_CLOSE) != 0) {
350353
// Just return if we already destroyed the underlying connection.
351354
return;
352355
}
353-
// We need to set the connection to null now to guard against reentrancy.
356+
reantranceGuard |= IN_FORCE_CLOSE;
357+
354358
QuicheQuicConnection conn = connection;
355-
connection = null;
356359

357360
unsafe().close(voidPromise());
358361
// making sure that connection statistics is avaliable
@@ -378,6 +381,7 @@ void forceClose() {
378381
timeoutHandler.cancel();
379382
} finally {
380383
flushParent();
384+
connection = null;
381385
conn.free();
382386
}
383387
}
@@ -769,7 +773,7 @@ int streamSend(long streamId, ByteBuf buffer, boolean fin) throws ClosedChannelE
769773
}
770774

771775
void connectionSendAndFlush() {
772-
if (inFireChannelReadCompleteQueue || inHandleWritableStreams) {
776+
if (inFireChannelReadCompleteQueue || (reantranceGuard & IN_HANDLE_WRITABLE_STREAMS) != 0) {
773777
return;
774778
}
775779
if (connectionSend()) {
@@ -832,7 +836,7 @@ private boolean handleWritableStreams() {
832836
if (isConnDestroyed()) {
833837
return false;
834838
}
835-
inHandleWritableStreams = true;
839+
reantranceGuard |= IN_HANDLE_WRITABLE_STREAMS;
836840
try {
837841
long connAddr = connection.address();
838842
boolean mayNeedWrite = false;
@@ -880,7 +884,7 @@ private boolean handleWritableStreams() {
880884
}
881885
return mayNeedWrite;
882886
} finally {
883-
inHandleWritableStreams = false;
887+
reantranceGuard &= ~IN_HANDLE_WRITABLE_STREAMS;
884888
}
885889
}
886890

@@ -1072,11 +1076,11 @@ private boolean connectionSendSimple() {
10721076
* {@link Channel#flush()} at some point.
10731077
*/
10741078
private boolean connectionSend() {
1075-
if (isConnDestroyed() || inConnectionSend) {
1079+
if (isConnDestroyed() || (reantranceGuard & IN_CONNECTION_SEND) != 0) {
10761080
return false;
10771081
}
10781082

1079-
inConnectionSend = true;
1083+
reantranceGuard |= IN_CONNECTION_SEND;
10801084
try {
10811085
boolean packetWasWritten;
10821086
SegmentedDatagramPacketAllocator segmentedDatagramPacketAllocator =
@@ -1091,7 +1095,7 @@ private boolean connectionSend() {
10911095
}
10921096
return packetWasWritten;
10931097
} finally {
1094-
inConnectionSend = false;
1098+
reantranceGuard &= ~IN_CONNECTION_SEND;
10951099
}
10961100
}
10971101

@@ -1189,7 +1193,9 @@ void connectionRecv(InetSocketAddress sender, ByteBuf buffer) {
11891193
// See also https://github.com/cloudflare/quiche/issues/817
11901194
return;
11911195
}
1192-
inRecv = true;
1196+
1197+
reantranceGuard |= IN_RECV;
1198+
11931199
try {
11941200
ByteBuf tmpBuffer = null;
11951201
// We need to make a copy if the buffer is read only as recv(...) may modify the input buffer as well.
@@ -1280,12 +1286,12 @@ void connectionRecv(InetSocketAddress sender, ByteBuf buffer) {
12801286
}
12811287
}
12821288
} finally {
1283-
inRecv = false;
1289+
reantranceGuard &= ~IN_RECV;
12841290
}
12851291
}
12861292

12871293
void recv() {
1288-
if (inRecv || isConnDestroyed()) {
1294+
if ((reantranceGuard & IN_RECV) != 0 || isConnDestroyed()) {
12891295
return;
12901296
}
12911297

@@ -1296,13 +1302,13 @@ void recv() {
12961302
return;
12971303
}
12981304

1299-
inRecv = true;
1305+
reantranceGuard |= IN_RECV;
13001306
try {
13011307
recvDatagram();
13021308
recvStream();
13031309
} finally {
13041310
fireChannelReadCompleteIfNeeded();
1305-
inRecv = false;
1311+
reantranceGuard &= ~IN_RECV;
13061312
}
13071313
}
13081314

0 commit comments

Comments
 (0)