单例类公共方法应该同步吗？

Question

我有一个为我的应用程序抽象弹性搜索 API 的单例包装类。

public class ElasticSearchClient {    

    private static volatile ElasticSearchClient elasticSearchClientInstance;

    private static final Object lock = new Object();

    private static elasticConfig ; 

    /*
    ** Private constructor to make this class singleton
    */
    private ElasticSearchClient() {
    }

    /*
    ** This method does a lazy initialization and returns the singleton instance of ElasticSearchClient
    */
    public static ElasticSearchClient getInstance() {
        ElasticSearchClient elasticSearchClientInstanceToReturn = elasticSearchClientInstance;
        if (elasticSearchClientInstanceToReturn == null) {
            synchronized(lock) {
                elasticSearchClientInstanceToReturn = elasticSearchClientInstance;
                if (elasticSearchClientInstanceToReturn == null) {
                    // While this thread was waiting for the lock, another thread may have instantiated the clinet.
                    elasticSearchClientInstanceToReturn = new ElasticSearchClient();
                    elasticSearchClientInstance = elasticSearchClientInstanceToReturn;
                }
            }
        }
        return elasticSearchClientInstanceToReturn;
    }

    /*
    ** This method creates a new elastic index with the name as the paramater, if if does not already exists.
    *  Returns true if the index creation is successful, false otherwise.
     */
    public boolean createElasticIndex(String index) {
        if (checkIfElasticSearchIndexExists(index)) {
            LOG.error("Cannot recreate already existing index: " + index);
            return false;
        }
        if (elasticConfig == null || elasticConfig.equals(BatchConstants.EMPTY_STRING)) {
            loadElasticConfigFromFile(ELASTIC_CONFIG_FILE_NAME);
        }
        if (elasticConfig != null && !elasticConfig.equals("")) {
            try {
                HttpURLConnection elasticSearchHttpURLConnection = performHttpRequest(
                    ELASTIC_SEARCH_URL + "/" + index,
                    "PUT",
                    elasticConfig,
                    "Create index: " + index
                );

                return elasticSearchHttpURLConnection != null &&
                       elasticSearchHttpURLConnection.getResponseCode() == HttpURLConnection.HTTP_OK;
            } catch (Exception e) {
             LOG.error("Unable to access Elastic Search API. Following exception occurred:\n" + e.getMessage());
          }
        } else {
            LOG.error("Found empty config file");
        }
        return false;
    }

private void loadElasticConfigFromFile(String filename) {

    try {
        Object obj = jsonParser.parse(new FileReader(filename);
        JSONObject jsonObject = (JSONObject) obj;
        LOG.info("Successfully parsed elastic config file: "+ filename);
        elasticConfig = jsonObject.toString();
        return;
    } catch (Exception e) {
        LOG.error("Cannot read elastic config from  " + filename + "\n" + e.getMessage());
        elasticConfig = "";
    }
}

}

我有多个使用 ElasticSearchClient 的线程，如下所述

Thread1
ElasticSearchClient elasticSearchClient = ElasticSearchClient.getInstance()
elasticSearchClient.createElasticIndex("firstindex");

Thread2
ElasticSearchClient elasticSearchClient = ElasticSearchClient.getInstance()
elasticSearchClient.createElasticIndex("secondindex");

Thread3...

在我看来，单例类是线程安全的，但我不确定如果多个线程开始执行单例类的相同方法会发生什么。这有什么副作用吗？

注意：我知道上面的单例类不是反射和序列化安全的。

Answer 1

在你的特定实现中

if (checkIfElasticSearchIndexExists(index)) { //NOT THREAD SAFE
            LOG.error("Cannot recreate already existing index: " + index);
            return false;
        }
        if (elasticConfig == null || elasticConfig.equals(BatchConstants.EMPTY_STRING)) { //NOT THREAD SAFE
            loadElasticConfigFromFile(ELASTIC_CONFIG_FILE_NAME);
        }
        if (elasticConfig != null && !elasticConfig.equals("")) { //NOT THREAD SAFE

有 3 个点可能会导致比赛状况。

所以

单例类公共方法应该同步吗？

没有这样的规则——如果这些规则是线程安全的，那么就不需要额外的同步。在您的情况下，这些不是线程安全的，因此您必须使它们成为线程安全的，例如。制作

public synchronized boolean createElasticIndex

如果你关心并发写入单个索引，那么不要 - 这是一个 ElasticSearch 任务来正确处理并发写入（相信我 ES 会顺利处理）

什么不是线程安全的（指出 3 个地方）？具有并发 T1 和 T2：

1. checkIfElasticSearchIndexExists(index) 如果 T1 和 T2 将使用相同的索引名称，则两者都将通过此检查（我只假设这是一个休息调用——那是最糟糕的）
2. elasticConfig == null || elasticConfig.equals(BatchConstants.EMPTY_STRING) 第一次运行时，T1 和 T2 都将通过此测试，并且都将从文件中加载配置 - 可能不会产生影响，但它仍然是一个赛车条件
3. if (elasticConfig != null && !elasticConfig.equals("")) 与 2 + 相同的场景，如果由于内存模型，如果 elasticConfig 不是 volatile 它可以在 loadElasticConfigFromFile 完全初始化之前读取为“not null”。

2 和 3 可以通过双重检查锁定来修复（就像您在 getInstance() 中所做的那样，或者我宁愿将其移至实例初始化块 - 我认为构造函数是最好的。

至于更好地理解这种现象，您可以查看why a==1 && a==2 can evaluate to true

1 然而，由于调用和响应之间的延迟，问题更大，您得到了一个宽窗口，其中 2 个线程可以查询相同的索引并获得完全相同的响应 - 该索引不存在并尝试创建一个。