kohlmu-pivotal commented on pull request #5750:
URL: https://github.com/apache/geode/pull/5750#issuecomment-727668794


   Whilst the change LGTM... I cannot help but feel that the whole logic of 
(un)/locking of the `connectionLifeCycleLock` should really be a little more 
hidden. This locking logic proliferates throughout the whole 
`GatewaySenderEventRemoteDispatcher`. Which makes it way too easy to actually 
miss logic where the connection lock needs to be used.
   @boglesby @gesterzhou maybe we should be locking at moving all this logic to 
another location. 
   There is also no indication of WHY this lock is required? Is it protecting 
the accessing of `connection` properties or is it avoiding the accidental 
setting of the `connection` field with another instance? It is not clear. 


----------------------------------------------------------------
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]


Reply via email to