java - 同步块(synchronized block)中List的数据重叠

Question

我有一个方法:

private List<String> userCns = Collections.synchronizedList(new ArrayList<String>());
private List<String> recipients = Collections.synchronizedList(new ArrayList<String>());

public void sendEmailToLegalUsers() {
    try {
        synchronized (lock) {
            searchGroup();

            if(userCns.size() > 0) {
                for(String userCn : userCns) {
                    String mail = getUserMail(userCn);
                    if(mail != null) {
                        recipients.add(mail);
                    }
                }                   
            }

            String docName = m_binder.getLocal("docname");
            String docId = m_binder.getLocal("docid");
            String url = m_binder.getLocal("serverURL");

            if(recipients.size() > 0) {
                m_binder.addResultSet("LOI_EVIN_MAIL", getLoiEvinMailResultSet(docName, docId, url));

                for(String recipient : recipients) {
                    Log.info("Sending mail to: " + recipient);
                    InternetFunctions.sendMailToEx(recipient, "MH_LOI_EVIN_SEND_EMAIL", "Update Evin Law Compliance for the item: " + docName, m_service, true);
                }
            }
        }           
    } catch (Exception e) { 
        Log.info("Error occurred in LDAPSendMail: "+ e.getMessage());
    }   
}

现在可以从不同的线程调用此 sendEmailToLegalUsers 方法。我想知道锁定代码块是否是正确的方法，这样列表中就不会出现数据混淆的情况？

<小时/>

编辑:全类:

package com.edifixio.ldapsendmail.handlers;

import intradoc.common.Log;
import intradoc.data.DataResultSet;
import intradoc.server.InternetFunctions;
import intradoc.server.ServiceHandler;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Hashtable;
import java.util.List;
import java.util.Vector;

import javax.naming.Context;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
import javax.naming.directory.Attribute;
import javax.naming.directory.Attributes;
import javax.naming.directory.DirContext;
import javax.naming.directory.InitialDirContext;
import javax.naming.directory.SearchControls;
import javax.naming.directory.SearchResult;

public class LDAPSendMail extends ServiceHandler {

    private final Object lock = new Object();

    private String ldapURL;
    private String baseDN;
    private String groupDN;
    private String username;
    private String password;
    private DirContext context;

    private List<String> userCns = Collections.synchronizedList(new ArrayList<String>());
    private List<String> recipients = Collections.synchronizedList(new ArrayList<String>());

    public void sendEmailToLegalUsers() {
        try {
            synchronized (lock) {
                searchGroup();

                if(userCns.size() > 0) {
                    for(String userCn : userCns) {
                        String mail = getUserMail(userCn);
                        if(mail != null) {
                            recipients.add(mail);
                        }
                    }                   
                }

                String docName = m_binder.getLocal("docname");
                String docId = m_binder.getLocal("docid");
                String url = m_binder.getLocal("serverURL");

                if(recipients.size() > 0) {
                    m_binder.addResultSet("LOI_EVIN_MAIL", getLoiEvinMailResultSet(docName, docId, url));

                    for(String recipient : recipients) {
                        Log.info("Sending mail to: " + recipient);
                        InternetFunctions.sendMailToEx(recipient, "MH_LOI_EVIN_SEND_EMAIL", "Update Evin Law Compliance for the item: " + docName, m_service, true);
                    }
                }

                userCns.clear();
                recipients.clear();
            }           
        } catch (Exception e) { 
            Log.info("Error occurred in LDAPSendMail: "+ e.getMessage());
        }   
    }

    private String getUserMail(String userCn) throws NamingException {
        NamingEnumeration<SearchResult> searchResults = getLdapDirContext().search(userCn, "(objectclass=person)", getSearchControls());
        while (searchResults.hasMore()){
            SearchResult searchResult = searchResults.next();
            Attributes attributes = searchResult.getAttributes();
            Attribute mail = null;

            try {
                mail = attributes.get("mail");
            } catch (Exception e) {
                mail = null;
            }

            if(mail != null) {
                return (String)mail.get();
            }
        }
        return null;
    }

    private void searchGroup() throws NamingException {
        NamingEnumeration<SearchResult> searchResults = getLdapDirContext().search(groupDN, "(objectclass=groupOfUniqueNames)", getSearchControls());
        String searchGroupCn = getCNForBrand(m_binder.getLocal("brandId"), m_binder.getLocal("brandName"));

        while (searchResults.hasMore()) {
            SearchResult searchResult = searchResults.next();
            Attributes attributes = searchResult.getAttributes();
            Attribute groupCn = null;

            try {
                groupCn = attributes.get("cn");
            } catch (Exception e) {
                groupCn = null;
            }

            if(groupCn != null) {
                if(searchGroupCn.equals((String)groupCn.get())) {
                    Attribute uniqueMembers = attributes.get("uniqueMember");
                    for(int i = 0; i < uniqueMembers.size(); i++){
                        String uniqueMemberCN = (String) uniqueMembers.get(i);
                        userCns.add(uniqueMemberCN);
                    }
                    break;
                }
            }
        }
    }

    private DirContext getLdapDirContext() throws NamingException {
        if(context != null) {
            return context;
        }

        ldapURL = m_binder.getLocal("ldapUrl");
        baseDN = m_binder.getLocal("baseDN");
        groupDN = new StringBuilder().append("ou=").append(getAccountGroup(m_binder.getLocal("account"))).append(",").append("ou=groups,").append(baseDN).toString();
        username = m_binder.getLocal("username");
        password = m_binder.getLocal("password");

        Hashtable<String, String> environment = new Hashtable<String, String>();
        environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
        environment.put(Context.PROVIDER_URL, ldapURL);
        environment.put(Context.SECURITY_PRINCIPAL, username);
        environment.put(Context.SECURITY_CREDENTIALS, password);

        context = new InitialDirContext(environment);

        return context;
    }

    private SearchControls getSearchControls() {
        SearchControls searchControls = new SearchControls();       
        searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
        return searchControls;
    }

    private String getCNForBrand(String brandId, String brandName) {        
        String[] brandIdSplittedArray = brandId.split("/");
        return new StringBuilder().append(brandIdSplittedArray[0]).append("-").append(brandIdSplittedArray[1]).append("-").
                append(brandIdSplittedArray[2]).append("-").append(brandName.replaceAll("\\s","")).append("-LU").toString();
    }

    private String getAccountGroup(String account) {        
        return account.split("/")[1];
    }

    private DataResultSet getLoiEvinMailResultSet(String docName, String docId, String url) {
        DataResultSet resultSet = new DataResultSet(new String[]{"DOCNAME", "DOCID", "URL"});
        Vector<String> vector = new Vector<String>();
        vector.add(docName);
        vector.add(docId);              
        vector.add(url);
        resultSet.addRow(vector);
        return resultSet;
    }
}

Answer 1

什么是锁？您在其他地方使用它吗？通常您希望同步块(synchronized block)非常小。如果您在任何地方都使用 lock 作为通用锁，那么您可能会阻止线程在完全不相关的区域(即不存在共享资源争用的区域)中执行一些有用的工作。

其次，recipients 真的需要是实例变量吗？奇怪的是，您不断向收件人添加电子邮件，而不检查该电子邮件是否已存在于列表中。我也看不到任何清除我们的收件人的代码。所以这是一个潜在的问题。如果您每次都从头开始构建收件人，那么只需将其设为方法中的局部变量即可。如果您确实需要访问该数据，您可以随时将其从 userCns 中提取。

一旦将recipients设为局部变量，那么您只需使用userCns作为锁来同步:

synchronized(userCns) {
   ...
}

编辑:您的代码显示您仅使用recipients一次，并且在sendEmailToLegalUsers方法内。正如我所指出的，另一件事是您从未清除收件人，因此这是代码中的错误。由于您不在任何地方使用 recipients，因此请将其作为 sendEmailToLEgalUsers 的局部变量。另外，只需通过 userCns 进行同步即可。您不需要通过收件人进行同步；您可以在同步块(synchronized block)内创建它。