[GitHub] [geode] dschneider-pivotal commented on pull request #3371: GEODE-6152: Removed use of futures (optimized get) for proxy region.

["dschneider-pivotal (GitHub)" <gi...@apache.org>]

This finally block seems buggy in the case that "otherFuture" != null. In that case no other thread will see "thisFuture" because the putIfAbsent found "otherFuture". So calling thisFuture.set here is a waste of time.
But even worse, when we do the "this.getFutures.remove", we could be removing someone else's future from the map. I think it should use the flavor of ConcurrentMap remove here that says only remove if if it is equal to "thisFuture" and this whole finally block should be protected by "if (otherFuture == null". 

[ Full content available at: https://github.com/apache/geode/pull/3371 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org