start(int) method initializes failureDetector (resp. staticMembers) only if TcpFailureDetector (resp. StaticMembershipInterceptor) was found in channel interceptors stack. Without TcpFailureDetector (resp. StaticMembershipInterceptor), futur calls to sendPing() will fail because failureDetector (resp. staticMembers) wasn't initialized at least to new WeakReference<StaticMembershipInterceptor>(null). Fix: v1) initializes weak references containers: Replace: WeakReference<TcpFailureDetector> failureDetector = null; WeakReference<StaticMembershipInterceptor> staticMembers = null; for: WeakReference<TcpFailureDetector> failureDetector = new WeakReference<TcpFailureDetector>(); WeakReference<StaticMembershipInterceptor> staticMembers = new WeakReference<StaticMembershipInterceptor>(); v2) checks field before dereferencing it: sendPing becomes: protected void sendPing() { if (failureDetector!=null && failureDetector.get()!=null) { //we have a reference to the failure detector //piggy back on that dude failureDetector.get().checkMembers(true); }else { if (staticOnly && staticMembers!=null && staticMembers.get()!=null) { sendPingMessage(staticMembers.get().getMembers()); } else { sendPingMessage(getMembers()); } } } affect also tomcat 6 regards fred arnoud
Sorry: Forget first solution (v1) we cannot set a weak reference. Need to modify sendPing method to check null pointer. regards fred
I used this solution: protected void sendPing() { TcpFailureDetector tcpFailureDetector = failureDetector!=null ? failureDetector.get() : null; if (tcpFailureDetector!=null) { //we have a reference to the failure detector //piggy back on that dude tcpFailureDetector.checkMembers(true); }else { StaticMembershipInterceptor staticMembershipInterceptor = staticOnly && staticMembers!=null ? staticMembers.get() : null; if (staticMembershipInterceptor!=null) { sendPingMessage(staticMembershipInterceptor.getMembers()); } else { sendPingMessage(getMembers()); } } }
(In reply to comment #2) > I used this solution: > > protected void sendPing() { > TcpFailureDetector tcpFailureDetector = failureDetector!=null ? > failureDetector.get() : null; > if (tcpFailureDetector!=null) { That's nice; and better than v2 as it also protects against another possible NPE, i.e.: failureDetector.get() can return null (it's a WeakReference, so get() can return null at any time).
You're right, only one access to get() for WeakReference (and brother classes).
Fixed in trunk and 7.0.x and will be included in 7.0.30 onwards. Thanks for the patch.
Whoops. Re-open for Tomcat 6.
Fixed in 6.0.x and will be included in 6.0.36 onwards.