[
https://issues.apache.org/jira/browse/GEODE-8651?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17220854#comment-17220854
]
ASF GitHub Bot commented on GEODE-8651:
---------------------------------------
dschneider-pivotal commented on pull request #5665:
URL: https://github.com/apache/geode/pull/5665#issuecomment-716704688
As far as the extra synchronization goes in readAck, I agree that if the
only place that was concurrently modifying this buffer was the extra
notifyHandshakeWaiter then this sync is not needed. But from the viewpoint of
readAck, it currently calls readHeader and readMessage or readChunk. All these
methods end up synchronizing to protect the buffer. If that sync is needed then
it is also needed at the readAck level. readAck is incorrect to allow another
thread to modify the buffer between the readHeader and readMessage calls. It is
hard to prove that no other threads could concurrently modify the buffer which
is why I think we added the new ioFilter.getSynchObject. I'm arguing that if we
are going to have getSynchObject then readAck should synchronize on it.
----------------------------------------------------------------
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:
[email protected]
> 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)
