Bug 53606 - NullPointerException in TcpPingInterceptor
Summary: NullPointerException in TcpPingInterceptor
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Cluster (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-26 16:15 UTC by F.Arnoud
Modified: 2012-08-27 21:35 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description F.Arnoud 2012-07-26 16:15:05 UTC
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
Comment 1 F.Arnoud 2012-07-26 16:21:11 UTC
Sorry:

Forget first solution (v1) we cannot set a weak reference.
Need to modify sendPing method to check null pointer.

regards
fred
Comment 2 F.Arnoud 2012-07-26 16:26:39 UTC
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());
            }
        }
    }
Comment 3 Sebb 2012-07-26 18:00:00 UTC
(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).
Comment 4 F.Arnoud 2012-07-26 18:36:18 UTC
You're right, only one access to get() for WeakReference (and brother classes).
Comment 5 Mark Thomas 2012-07-29 21:52:02 UTC
Fixed in trunk and 7.0.x and will be included in 7.0.30 onwards. Thanks for the patch.
Comment 6 Mark Thomas 2012-07-29 21:54:46 UTC
Whoops. Re-open for Tomcat 6.
Comment 7 Mark Thomas 2012-08-27 21:35:31 UTC
Fixed in 6.0.x and will be included in 6.0.36 onwards.