Bill commented on a change in pull request #6100:
URL: https://github.com/apache/geode/pull/6100#discussion_r591992420
##########
File path:
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
##########
@@ -1456,12 +1456,18 @@ void
processNetworkPartitionMessage(NetworkPartitionMessage<ID> msg) {
return;
}
ID sender = msg.getSender();
- if (getView().getMembers().contains(sender)) {
- String str = "Membership coordinator " + msg.getSender()
- + " has declared that a network partition has occurred";
- forceDisconnect(str);
+
+ if (getView() != null && isJoined) {
+ if (getView().getMembers().contains(sender)) {
Review comment:
It appears to me that the only way `currentView` can be `null` is before
the first call to `installView(GMSMembershipView<ID>)`. It is TBD if that
method is ever called with a `null` reference. If we could ensure that it was
never called with a `null` reference then the null-check in the current PR
might be ok, since that would mean `currentView` goes from `null` to non-null
and then never goes to `null` again.
I have done some analysis on `installView(GMSMembershipView<ID>)` by adding
the `@NotNull` annotation to `Service.installView(@NotNull
GMSMembershipView<ID> v)` and then looking for violations found by IntelliJ
inspections. Of the 6 usages outside tests, IntelliJ's inspections doesn't flag
any of them as violations.
Since I'm so new to using this inspection, I don't quite trust it (or my
interpretation of it.) I wonder if it might be a good idea to make the code
here a little more defensive by capturing the `currentView` here, and then
operating on that. Like:
```
final GMSMembershipView<ID> view = getView();
if (view != null && isJoined) {
if (view.getMembers().contains(sender)) {
```
That'd ensure that any changes to `currentView` between the two calls to
`getView()` don't bite us.
----------------------------------------------------------------
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]
