1

我有一个方法：

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;
    }
}
4

2 回答 2

1

是什么lock？你在其他地方使用它吗？通常，您希望同步块非常小。如果您将lock任何地方用作通用锁，那么您可能会阻止线程在完全不相关的区域（即，没有争用共享资源的区域）做一些有用的工作。

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

一旦你创建recipients了一个局部变量，那么你只需要使用userCns锁来同步：

synchronized(userCns) {
   ...
}

编辑：您的代码显示您只使用recipients一次，并且在sendEmailToLegalUsers方法内部。正如我所指出的，另一件事是您永远不会清除recipients，因此这是您的代码中的错误。由于您不在recipients任何地方使用，请将其设为sendEmailToLEgalUsers. 此外，只需同步userCns. 您不需要同步recipients；您可以在同步块中创建它。

于 2012-08-29T15:46:19.137 回答
1

我会做

private final List<String> userCns = new ArrayList<String>();
private final List<String> recipients = new ArrayList<String>();

和

synchronized(userCns) {
   // as Vivin suggests.
}

你不需要额外的锁。

于 2012-08-29T16:07:44.087 回答