Superposición de datos de la lista en bloque sincronizado

Tengo un metodo:

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());
    }   
}

Ahora este método sendEmailToLegalUsers se puede llamar desde diferentes hilos. Me pregunto si es la forma correcta de bloquear el bloque de código para que no haya posibilidades de confusión de datos en la lista.


Editar: toda la clase:

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;
    }
}
Respuesta 1

¿Qué es lock? ¿Lo estás usando en otro lado? Por lo general, desea que los bloques sincronizados sean bastante pequeños. Si está utilizando en locktodas partes como un bloqueo de propósito general, entonces podría estar evitando que un hilo haga un trabajo útil en un área totalmente no relacionada (es decir, una donde no hay contención por los recursos compartidos).

Segundo, ¿ recipientsrealmente necesita ser una variable de instancia? Parece extraño que continúe agregando correos electrónicos recipientssin verificar si ese correo electrónico ya existe en la lista. Tampoco puedo ver ningún código donde estás borrando nuestro recipients. Entonces ese es un problema potencial. Si va a construir recipientsdesde cero cada vez, simplemente conviértalo en una variable local en el método. Si realmente necesita acceso a esos datos, siempre puede extraerlos userCns.

Una vez que crea recipientsuna variable local, solo necesita sincronizar usando userCnscomo un bloqueo:

synchronized(userCns) {
   ...
}

editar: su código muestra que solo lo usa recipientsuna vez, y eso está dentro del sendEmailToLegalUsersmétodo. Otra cosa, como señalé, es que nunca se borra, recipientspor lo que es un error en su código. Ya que no usasrecipients ninguna parte, conviértalo en una variable local para sendEmailToLEgalUsers. Además, solo sincronice userCns. No necesitará sincronizar de nuevo recipients; puedes crearlo dentro del bloque sincronizado.

Respuesta: 2

Estoy creando una conexión db con Oracle usando JDBC en una página JSP. ¿Es seguro llamar a la conexión cada vez antes de consultar, ya que las páginas contienen paginación del lado del servidor y también algunas de inserción / eliminación ...

El comportamiento estándar de Eclipse workbench es preservar el conjunto de archivos abiertos entre invocaciones e intentar volver a abrir esos mismos archivos al reiniciar. Si falta alguno de los archivos, entonces ...

¿Cómo imprimo un marco de datos completo en Java sin quedarse sin memoria? Conjunto de datos <Row> df = ... Sé que: df.show () mostrará el marco de datos, pero con un marco de datos lo suficientemente grande es ...

Tengo un fragmento HTML simple similar a este: <a href="123"> link </a> Necesito transformarlo en <abc: href var = "123"> link </ abc: href> Lo hago con XSLT, así que tuve que agregar el ...