fortify给代码的正确性双重检查锁定问题的代码

Question

Class MnetLdapHelper {

    private static volatile MnetLDAPService ldapSvc = null;
    public MnetLdapHelper() throws Throwable {
        if (ldapSvc == null) {
            synchronized(MnetLdapHelper.class) {
                if (ldapSvc == null) {
                    setup();
                }
            }
        }
    }
    public void setup() throws Throwable {
        PropertyManager appProp = PropertyUtil.getInstance()
            .getPropertyManager();
        try {

            sLog.info("MnetLdapHelper setup(+) ");
            ldapHost = appProp.getStringProperty("LDAP", "Host");
            ldapSslPOrt = Integer.valueOf(appProp.getStringProperty("LDAP", "Port"));
            ldapPort = Integer.valueOf(appProp.getStringProperty("LDAP", "NonSSLPort"));
            baseDn = appProp.getStringProperty("LDAP", "BaseDN");
            appCUID = appProp.getStringProperty("LDAP", "AppCUID");
            appPasswd = appProp.getStringProperty("LDAP", "AppPassword");
            groupBaseDn = appProp.getStringProperty("LDAPGROUPS", "GroupBaseDN");

            String appDN = null;
            int minConnInt = 1;
            int maxConnInt = 10;
            if (minConn != null && minConn.length() > 0) {
                minConnInt = Integer.parseInt(minConn);
            }
            if (maxConn != null && maxConn.length() > 0) {
                maxConnInt = Integer.parseInt(maxConn);
            }

            ldapSvc = new MnetLDAPService(ldapHost, ldapPort, false, null,
                null, baseDn, minConnInt, maxConnInt);

            LDAPEntry appEntry = ldapSvc.getUser(appCUID, null, null);
            if (appEntry == null) {
                sLog.error("Non-existent application CUID: [" + appCUID +
                    "], throw new Exception...");
                throw new Exception("Non-existent application CUID: [" +
                    appCUID + "]");
            }
            appDN = appEntry.getDN();
            sLog.info("appDN is: " + InfoSecValidation.cleanLogMessage(appDN));

            // Empty the non-SSL connection pool
            ldapSvc.finalize();

            ldapSvc = new MnetLDAPService(ldapHost, ldapSslPOrt, true, appDN,
                appPasswd, baseDn);
            // ldapSvc.setDisconnect(Boolean.parseBoolean(ldapApiDisconnectConnectionFlag));
            ldapSvc.setDisconnect(false);
            // ldapSvc.setUserSearchAttributes(IDATTRS);

        } catch (LDAPException ldEx) {
            if (ldapSvc != null) {
                ldapSvc.finalize();
                ldapSvc = null;
            }
            sLog.error(
                "LDAPException caught in setup(), throw new DocsServiceException... " +
                ldEx.errorCodeToString(), ldEx);
            throw ldEx;

        } catch (Throwable ex) {
            if (ldapSvc != null) {
                ldapSvc.finalize();
                ldapSvc = null;
            }
            sLog.error(
                "Throwable caught in setup(), throw new DocsServiceException",
                ex);
            throw ex;    
        }
    }    
}

以上这段代码被用在我的应用程序中。以前，我得到的代码正确性：双重检查锁定fortify问题时，我没有使用volatile，然后我发现，我们可以解决这个问题，使用volatile关键字，但问题仍然存在，有人能帮助我这里有什么问题吗？

Answer 1

你的解决方案有一个缺陷。 问题在于 setup() 方法可以在任何时候被任何线程调用，而且它通过副作用改变了私有静态的状态。

一个更好的解决方案是改变 setup() 创建并返回一个 MnetLdapHelper 实例，并将其结果赋值给 static.

class MnetLdapHelper {

    private static  volatile MnetLDAPService ldapSvc = null;

    public MnetLdapHelper() {
        if (ldapSvc == null) {
            synchronized (MnetLdapHelper.class) {
                if (ldapSvc == null) {
                    ldapSvc = setup();
                }
            }
        }
    }

    private MnetLDAPService setup() {
        MnetLDAPService res = ...
        // initialize it
        return res;
    }
}

现在值得商榷的是，Fortify的将你的版本报告为不正确执行的双重检查锁是否正确。 可能是Fortify使用的启发式方法不正确。

但如果你能重写代码以满足Fortify的要求，那就没有意义了。 我想你会发现我的版本可以做到这一点。

---

我也会对任何被声明为的方法或构造函数提出质疑。throws Throwable. 你应该把它改成列表 只是 勾选的异常情况，你 期待 要扔掉。

最后，你使用的方式是 static 这里很有可能是你造成单元测试的问题 MnetLdapHelper 或其他使用 MnetLdapHelper 例如：