Skip to content

Commit 4bfe0db

Browse files
ouharrispring-builds
authored andcommitted
GH-10671: Fix writingLatch lifecycle in TcpNioConnection
Inside `TcpNioConnection.ChannelInputStream.write()`, the implementation currently performs: ``` TcpNioConnection.this.writingLatch = new CountDownLatch(1); ``` This line replaces the latch with a new one after every write, even when the assembler is already waiting on the previous latch. This leads to a lost‑notification race: * Assembler enters `convert()`, captures latch A, and prepares to wait. * Before the assembler actually calls `await()`, the `ChannelInputStream.write()` executes and replaces latch A with B. * NIO reader signals B, but assembler is still waiting on A. * A is never signaled → assembler waits the full 60s → Timed out waiting for IO. * While assembler is blocked, the pipe cannot be drained → queue fills → Timed out waiting for buffer space. Fix a lost-notify race in `TcpNioConnection` where `writingLatch` could be replaced from `ChannelInputStream.write()`, causing the assembler to wait on a stale latch under concurrency. Changes: Do not replace `writingLatch` from the pipe write path Clear `writingLatch` at the end of the `doRead()` cycle Add a regression test proving the latch is not replaced during pipe write and is cleared after read Signed-off-by: Outman Ouharri <ouharri.outman@gmail.com> (cherry picked from commit 18c72d1)
1 parent 047aea4 commit 4bfe0db

2 files changed

Lines changed: 57 additions & 1 deletion

File tree

spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/TcpNioConnection.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@ private void doRead() throws IOException {
483483
finally {
484484
this.writingToPipe = false;
485485
writingLatchToUse.countDown();
486+
this.writingLatch = null;
486487
}
487488
}
488489

@@ -828,7 +829,6 @@ public void write(ByteBuffer byteBuffer) throws IOException {
828829
Thread.currentThread().interrupt();
829830
throw new IOException("Interrupted while waiting for buffer space", e);
830831
}
831-
TcpNioConnection.this.writingLatch = new CountDownLatch(1);
832832
}
833833
}
834834

spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/TcpNioConnectionTests.java

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,62 @@ public void testNoMultiAccept() throws InterruptedException, IOException {
798798
testMulti(false);
799799
}
800800

801+
@Test
802+
public void testWritingLatchClearedAfterRead() throws Exception {
803+
SocketChannel channel = mock();
804+
Socket socket = mock();
805+
when(channel.socket()).thenReturn(socket);
806+
when(socket.getReceiveBufferSize()).thenReturn(1024);
807+
808+
doAnswer(invocation -> {
809+
ByteBuffer buffer = invocation.getArgument(0);
810+
buffer.put("foo".getBytes());
811+
return 3;
812+
}).when(channel).read(Mockito.any(ByteBuffer.class));
813+
814+
TcpNioConnection connection = new TcpNioConnection(channel, false, false, this.nullPublisher, null);
815+
816+
CompositeExecutor compositeExec = compositeExecutor();
817+
connection.setTaskExecutor(compositeExec);
818+
819+
DirectFieldAccessor dfa = new DirectFieldAccessor(connection);
820+
821+
ChannelInputStream originalStream =
822+
TestUtils.getPropertyValue(connection, "channelInputStream", TcpNioConnection.ChannelInputStream.class);
823+
assertThat(originalStream).isNotNull();
824+
825+
ChannelInputStream streamSpy = spy(originalStream);
826+
dfa.setPropertyValue("channelInputStream", streamSpy);
827+
828+
AtomicReference<CountDownLatch> latchSeenDuringWrite = new AtomicReference<>();
829+
830+
doAnswer(invocation -> {
831+
CountDownLatch currentLatch = (CountDownLatch) dfa.getPropertyValue("writingLatch");
832+
if (currentLatch != null) {
833+
latchSeenDuringWrite.compareAndSet(null, currentLatch);
834+
}
835+
return invocation.callRealMethod();
836+
}).when(streamSpy).write(any(ByteBuffer.class));
837+
838+
Method doRead = TcpNioConnection.class.getDeclaredMethod("doRead");
839+
doRead.setAccessible(true);
840+
841+
doRead.invoke(connection);
842+
843+
assertThat(latchSeenDuringWrite.get())
844+
.as("writingLatch should be non-null while data is written to the pipe")
845+
.isNotNull();
846+
847+
CountDownLatch writingLatchAfterRead = (CountDownLatch) dfa.getPropertyValue("writingLatch");
848+
assertThat(writingLatchAfterRead)
849+
.as("writingLatch must be null after a completed read cycle")
850+
.isNull();
851+
852+
connection.close();
853+
854+
cleanupCompositeExecutor(compositeExec);
855+
}
856+
801857
private static void testMulti(boolean multiAccept) throws InterruptedException, IOException {
802858
CountDownLatch serverReadyLatch = new CountDownLatch(1);
803859
CountDownLatch latch = new CountDownLatch(21);

0 commit comments

Comments
 (0)