[
https://issues.apache.org/jira/browse/GEODE-8651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17221607#comment-17221607
]
ASF GitHub Bot commented on GEODE-8651:
---------------------------------------
dschneider-pivotal commented on a change in pull request #5665:
URL: https://github.com/apache/geode/pull/5665#discussion_r512899430
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/tcp/ConnectionTest.java
##########
@@ -113,4 +114,35 @@ public void connectTimeoutIsShortWhenAlerting() throws
UnknownHostException {
assertThat(connection.getP2PConnectTimeout(distributionConfig)).isEqualTo(100);
});
}
+
+ @Test
+ public void notifyHandshakeWaiterShouldPositionByteBufferOnlyOnce() throws
Exception {
+ ConnectionTable connectionTable = mock(ConnectionTable.class);
+ Distribution distribution = mock(Distribution.class);
+ DistributionManager distributionManager = mock(DistributionManager.class);
+ DMStats dmStats = mock(DMStats.class);
+ CancelCriterion stopper = mock(CancelCriterion.class);
+ SocketCloser socketCloser = mock(SocketCloser.class);
+ TCPConduit tcpConduit = mock(TCPConduit.class);
+
+ when(connectionTable.getBufferPool()).thenReturn(new BufferPool(dmStats));
+ when(connectionTable.getConduit()).thenReturn(tcpConduit);
+ when(connectionTable.getDM()).thenReturn(distributionManager);
+ when(connectionTable.getSocketCloser()).thenReturn(socketCloser);
+ when(distributionManager.getDistribution()).thenReturn(distribution);
+ when(stopper.cancelInProgress()).thenReturn(null);
+ when(tcpConduit.getCancelCriterion()).thenReturn(stopper);
+ when(tcpConduit.getDM()).thenReturn(distributionManager);
+ when(tcpConduit.getSocketId()).thenReturn(new
InetSocketAddress(getLocalHost(), 10337));
+ when(tcpConduit.getStats()).thenReturn(dmStats);
+
+ SocketChannel channel = SocketChannel.open();
+
+ Connection connection = new Connection(connectionTable, channel.socket());
+ connection = spy(connection);
+ connection.setSharedUnorderedForTest();
+ connection.run();
+
+ verify(connection, times(1)).getConduit();
Review comment:
I don't like that we verify that getConduit is called 1 time. I
understand after looking at the code but it seems rather indirect logically and
brittle. I would prefer that you extract the block of code in
notifyHandshakeWaiter starting at line 810 into a method named
"clearSSLInputBuffer". Then this test can assert that clearSSLInputBuffer is
only called 1 time. This would be much clearer and less brittle and would not
increase the diffs much
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> MsgReader's readHeader and readMessage should be synchronized
> -------------------------------------------------------------
>
> Key: GEODE-8651
> URL: https://issues.apache.org/jira/browse/GEODE-8651
> Project: Geode
> Issue Type: Bug
> Components: membership
> Reporter: Xiaojian Zhou
> Assignee: Xiaojian Zhou
> Priority: Major
> Labels: GeodeOperationAPI, pull-request-available
>
> When the connection reads direct ack, there're 2 steps: readHeader and
> readMessage.
> Another thread could jump in between to clear the buffer.
> The another thread is doing notifyHandShakeWaiter's second call, which could
> clear the buffer.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
