pivotal-jbarrett commented on a change in pull request #5947:
URL: https://github.com/apache/geode/pull/5947#discussion_r564814163
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
##########
@@ -606,38 +605,37 @@ boolean processHandShake() {
return result;
}
} finally {
- if (isTerminated() || !result) {
- return false;
- }
- boolean registerClient = false;
- synchronized (getCleanupProxyIdTable()) {
- MutableInt numRefs = getCleanupProxyIdTable().get(proxyId);
- if (numRefs != null) {
- numRefs.increment();
- } else {
- registerClient = true;
- getCleanupProxyIdTable().put(proxyId, new MutableInt(1));
+ if (!isTerminated() && result) {
Review comment:
It raises a good question. Returning or throwing form a finally block is
strongly discouraged. Given the lack of comments it was hard to infer the
intent. Given your concern that it may have been intentionally discarding
exceptions there is enough reasonable doubt to undo this change. It needs more
investigation to understand the intentions.
----------------------------------------------------------------
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]
