[hyddd的FindBugs分析记录][M C RCN] Nullcheck of value previously dereferenced

[M C RCN] Nullcheck of value previously dereferenced [RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE]

A value is checked here to see whether it is null, but this value can't be null because it was previously dereferenced and if it were null a null pointer exception would have occurred at the earlier dereference. Essentially, this code and the previous dereference disagree as to whether this value is allowed to be null. Either the check is redundant or the previous dereference is erroneous.

 

先看一段代码：

public class MyTest {
     
    private String str = "123";
    public void setStr(String str){
        this.str = str;
    }
    public String getStr(){
        return this.str;
    }

    public void test(){
        String str2 = "123";
        synchronized (str) {
            if (str != null) {
                str2 ="123";
            } else {
                str2 ="456";
            }           
            System.out.println(str2);
        }
    }
}

这个时候这段代码就会报Nullcheck of value previously dereferenced这个Bug，看Bug定位，发现问题出现在synchronized (str) 这里，str没有检查是否为NULL?!OK，我现在改用getStr()这个函数代替直接使用str，即：synchronized (getStr())，重新FindBug......居然没有发现错误-_-，但事实上getStr()并没有检查str是否为Null！！

　　现在我换另外一种写法，代码如下：

public class MyTest {

    private String str = "123";
    public void setStr(String str){
        this.str = str;
    }
    public String getStr(){
        return this.str;
    }

    public void test(){
        String str2 = "123";

        if(str != null){　　　　　　　　//tag2
            synchronized (str) {
                if (str != null) {　　　　//tag1
                    str2 ="123";
                } else {
                    str2 ="456";
                }           
                System.out.println(str2);
            }
        }
        
    }
}

这次我在tag2处加了一行检查str是否为NULL的代码，看FindBugs结果，出现了另外一个中等BUG：[M D RCN]Redundant nullcheck of value known to be non-null，跟踪发现是tag1处的代码是多余的，因为tag2处已经检查了一遍，并且在synchronized (str)后，str被独占，它不可能被修改，也就是说synchronized (str)后，根本不需要检查str是否为空，tag1处的代码是多余的。如果把tag1处代码去掉，[M D RCN]警告就没有了。

　　不知道大家有没有发现，上面的代码还有个问题，看这段代码：

if(str != null){
        synchronized (str) {      
            System.out.println(“123”);
        }
}

如果是多线程运行时，你不能排除它会if(str != null)和synchronized(str)之间进行线程切换，然后把str至为null！所以上面这样写其实也是有可能出现问题的！只是FindBugs没有找出来。

我觉得最好的写法还是：

    public void test() throws Exception{
        try{
            String str2 = "123";
            synchronized (getStr()) {
                str2 ="456";
                System.out.println(str2);
            }
        }
        catch(Exception ex){
            throw ex;
        }
　　　//Do other things....
    }

其实synchronized (getStr()) 换成synchronized (str) 也是可以的，FindBugs不会再报Bug了。

 

　　总结一下，我觉得FindBugs之所以会报[H C RCN] Nullcheck of value previously dereferenced，是因为我没有检查str的值是否为Null，并且没有注意对可能出现的Exception的截获。而简单使用getStr()，不检查str的值，不作异常捕获，也能躲过这个Bug，我觉得可能是FindBugs的一个BUG:<，因为估计它会认为，getStr()这个函数里面会检查str的值......但这个仅仅是个人认为而已。