jueves
nov082012
Problema con iteración
chocesSe sabe que los iteradores no son thread-safe. Lo mismo pueden lanzar una ConcurrentModificationException, que mostrar resultados inconsistentes, en un entorno multitarea.
Me he encontrado con el siguiente problema:
@Override
public List<String> getCovers(final List<Release> releases) {
final List<String> coversURL = new ArrayList<>(2 * releases.size());
for (final Iterator<Release> it = releases.iterator(); it.hasNext();) {
coversURL.add(it.next().getThumb());
}
return coversURL;
}
Este método forma parte de una clase que funciona como Service Provider, y al que se accede mediante un interface.
La cuestión es que si una tarea paralela modifica la lista releases, mientras que otra diferente ejecuta este método, se producirá o una excepción (lo más probable) o una inconsistencia extraña.
De momento, la única solución que he encontrado sería:
@Override
public List<String> getCovers(final List<Release> releases) {
final ArrayList<Release> localReleases = new ArrayList<>(releases);
final List<String> coversURL = new ArrayList<>(2 * localReleases.size());
for (final Iterator<Release> it = localReleases.iterator(); it.hasNext();) {
coversURL.add(it.next().getThumb());
}
return coversURL;
}
que realiza una "copia defensiva" del parámetro, para forzar la iteración sobre la variable local. El objeto es evitar una excepción, porque si el parámetro cambia, los resultados no serán consistentes con el cambio.
¿Hay otras soluciones?.
Reader Comments (10)
No me parece mala solución, si sabes que la lista nunca va a ser tan grande que pueda producir problemas de memoria. Igual utilizando alguna lista sincronizada que existen ahora se pudiera hacer algo.
Saludos
Con una lista sincronizada el problema sería el mismo, porque ni siquiera sus iteradores son thread-safe.
El asunto de la memoria es el habitual en Java: hace falta hacer pruebas para dimensionar adecuadamente el Heap. En caso de duda, y si las pruebas no son concluyentes, no queda más remedio que usar los Direct Buffers.
Por lo menos, la copia defensiva se libera en cuanto finalice el método, por lo que el consumo queda circunscrito al tiempo de ejecución del método.
Choces, ¿y la posibilidad de sincronizar el método? ¿No sería más sencillo? Así aseguras que el método es ejecutado por un solo hilo a la vez, evitando problemas de inconsistencia.
Saludos
La sincronización del método solamente afecta al bloque de código sincronizado; pero no impide que el contenido del parámetro pueda ser modificado por otra tarea.
Es un problema similar al que se presenta con la encapsulación de campos de una clase, cuando los parámetros son arrays o colecciones.
@Override
public List<String> getCovers(final List<Release> releases) {
synchronized (releases) {
final List<String> coversURL = new ArrayList<>(2 * releases.size());
for (final Iterator<Release> it = releases.iterator(); it.hasNext();) {
coversURL.add(it.next().getThumb());
}
return coversURL;
}
}
En este caso, incluso cuando el bloque de código está sincronizado usando el parámetro como monitor, no hay nada que impida que el contenido de releases cambie en origen, mientras el bloque sincronizado se ejecuta.
Es un caso complejo. Si la lista es pequeña lo mejor no es ya hacer copias defensivas, si no usar objetos inmutables. Otra opcion seria usar un sisteam de copyOnWrite (imagino que sera lo que CopyOnWriteArrayList haga, pero nunca la use).
Por otro lado si la lista es muy, muy grande, tanto que no puedas tenerla duplicada en memoria, lo mejor seria, como dicen, sincronizarla. Pero sincronizarla bien. Es decir, ocultarla detras de un objeto que controle el acceso concurrente de tal manera que no se produzca el problema!
Pero solo haria eso si realmente no pudiera permitirme copiarlo en memoria, no por razones de "uf, paso de crear memoria que es muy caro", porque ciertamente los recolectores de basura son mas eficientes de lo que nos creemos y sin duda mas eficiente que los cerrojos!
No se puede asegurar que el parámetro de un método, en un API público, vaya a ser inmutable por definición.
En su declaración: public List<String> getCovers(final List<Release> releases) no se establece ninguna característica específica, excepto que se trata de una List.
CopyOnWriteArrayList no garantiza que los iteradores sean thread-safe, por lo que usar esa clase como tipo del parámetro no resuelve el problema.
Me gustaría ver el código que asegura una correcta sincronización de un parámetro de un método. El API público al que pertenece ese método, no puede asumir nada sobre la correcta o incorrecta sincronización del mismo, en las instancias donde se declara.
Creo entender que con la solución que aportas no evitas que salte la excepción "ConcurrentModificationException".
En el momento en que creas la copia de respaldo:
final ArrayList<Release> localReleases = new ArrayList<>(releases);
entiendo que internamente se efectúa una lectura de la lista para crearla, si al mismo tiempo "otra tarea paralela" modifica la lista salta la excepción.
Por lo que pude leer y entender, para evitar el salto de la excepción debemos declarar la lista como "CopyOnWriteArrayList" para que pueda ser modificada desde diferentes "tareas paralelas".
Texto extraído del libro "Piensa en Java":
"En CopyOnWriteArrayList, una escritura hará que se cree una copia de toda la matriz subyacente.La matriz original sigue existiendo para que puedan realizarse lecturas seguras mientras se está modificando la matriz copiada...........etc.........no genera excepciones ConcurrentModificationException cuando hay múltiples iteradores recorriendo y modificando la lista, etc...."
La solución que aportas de crear la copia de respaldo va en la dirección que se siguió con la creación de "CopyOnWriteArrayList".
Esta afirmación del post anterior es errónea:
"" Creo entender que con la solución que aportas no evitas que salte la excepción "ConcurrentModificationException".
En el momento en que creas la copia de respaldo:
final ArrayList<Release> localReleases = new ArrayList<>(releases);
entiendo que internamente se efectúa una lectura de la lista para crearla, si al mismo tiempo "otra tarea paralela" modifica la lista salta la excepción""
Si se observan ambos constructores, de ArrayList y de CopyOnWriteArrayList:
public ArrayList(Collection<? extends E> c) {
elementData = c.toArray();
size = elementData.length;
// c.toArray might (incorrectly) not return Object[] (see 6260652)
if (elementData.getClass() != Object[].class)
elementData = Arrays.copyOf(elementData, size, Object[].class);
}
public CopyOnWriteArrayList(Collection<? extends E> c) {
Object[] elements = c.toArray();
// c.toArray might (incorrectly) not return Object[] (see 6260652)
if (elements.getClass() != Object[].class)
elements = Arrays.copyOf(elements, elements.length, Object[].class);
setArray(elements);
}
se ve que no hay ninguna sincronización especial con respecto a la creación del array interno.Lo que es lógico puesto que la ejecución de los constructores está controlada por la VM.
No veo ninguna diferencia entre usar una u otra clase en lo que se refiere a la inicialización, que es el asunto que nos ocupa aquí.
Con respecto a si lanza o no ConcurrentModificationException, ambos constructores usan
public Object[] toArray() {
return Arrays.copyOf(elementData, size);
}
donde el método copyOf puede lanzar lo que sigue, según su Javadoc;
* @throws NegativeArraySizeException if <tt>newLength</tt> is negative
* @throws NullPointerException if <tt>original</tt> is null
* @throws ArrayStoreException if an element copied from
* <tt>original</tt> is not of a runtime type that can be stored in
* an array of class <tt>newType</tt>
Las causas de esa excepción se explican bien en su Javadoc:
http://docs.oracle.com/javase/7/docs/api/java/util/ConcurrentModificationException.html