Asignar un objeto a un campo definido fuera de un bloque sincronizado: ¿es seguro para subprocesos?

Question

¿Hay algún problema con la seguridad del hilo de este código java? Los subprocesos 1-10 agregan números a través de sample.add(), y los subprocesos 11-20 llaman a removeAndDouble() e imprimen los resultados en stdout. Recuerdo que, en el fondo de mi mente, alguien dijo que asignar un elemento de la misma manera que tengo en removeAndDouble() usándolo fuera del bloque sincronizado puede no ser seguro para subprocesos. Que el compilador puede optimizar las instrucciones para que ocurran fuera de secuencia. ¿Es ese el caso aquí? ¿Mi método removeAndDouble() es inseguro?

¿Hay algo más erróneo desde una perspectiva de concurrencia con este código? Estoy tratando de obtener una mejor comprensión de la concurrencia y el modelo de memoria con java (1.6 en adelante).

import java.util.*; 
import java.util.concurrent.*; 

public class Sample { 

    private final List<Integer> list = new ArrayList<Integer>(); 

    public void add(Integer o) { 
     synchronized (list) { 
      list.add(o); 
      list.notify(); 
     } 
    } 

    public void waitUntilEmpty() { 
     synchronized (list) { 
      while (!list.isEmpty()) { 
       try { 
        list.wait(10000); 
       } catch (InterruptedException ex) { } 
      } 
     } 
    } 

    public void waitUntilNotEmpty() { 
     synchronized (list) { 
      while (list.isEmpty()) { 
       try { 
        list.wait(10000); 
       } catch (InterruptedException ex) { } 
      } 
     } 
    } 

    public Integer removeAndDouble() { 
     // item declared outside synchronized block 
     Integer item; 
     synchronized (list) { 
      waitUntilNotEmpty(); 
      item = list.remove(0); 
     } 
     // Would this ever be anything but that from list.remove(0)? 
     return Integer.valueOf(item.intValue() * 2); 
    } 

    public static void main(String[] args) { 
     final Sample sample = new Sample(); 

     for (int i = 0; i < 10; i++) { 
      Thread t = new Thread() { 
       public void run() { 
        while (true) { 
         System.out.println(getName()+" Found: " + sample.removeAndDouble()); 
        } 
       } 
      }; 
      t.setName("Consumer-"+i); 
      t.setDaemon(true); 
      t.start(); 
     } 

     final ExecutorService producers = Executors.newFixedThreadPool(10); 
     for (int i = 0; i < 10; i++) { 
      final int j = i * 10000; 
      Thread t = new Thread() { 
       public void run() { 
        for (int c = 0; c < 1000; c++) { 
         sample.add(j + c); 
        } 
       } 
      }; 
      t.setName("Producer-"+i); 
      t.setDaemon(false); 
      producers.execute(t); 
     } 

     producers.shutdown(); 
     try { 
      producers.awaitTermination(600, TimeUnit.SECONDS); 
     } catch (InterruptedException e) { 
      e.printStackTrace(); 
     } 

     sample.waitUntilEmpty();   
     System.out.println("Done."); 
    } 
} 

Answer 1

Su sincronización está bien, y no dará lugar a ningún problema de ejecución fuera de orden.

Sin embargo, veo algunos problemas.

En primer lugar, el método de waitUntilEmpty sería mucho más oportuna si se agrega un list.notifyAll() después de la list.remove(0) en removeAndDouble. Esto eliminará un retraso de hasta 10 segundos en su wait(10000).

segundo lugar, su list.notify en debe ser un notifyAll, porque notify sino estelas un hilo, y puede despertar a un hilo que está esperando dentro waitUntilEmpty en lugar de waitUntilNotEmpty.

En tercer lugar, ninguno de los anteriores es terminal a la vida de su aplicación, porque utilizó esperas limitadas, pero si realiza los dos cambios anteriores, su aplicación tendrá un mejor rendimiento (waitUntilEmpty) y las esperas limitadas serán innecesarias y pueden se convierten en simples y antiguas esperas no-arg.

Answer 2

Me parece seguro para hilos. Aquí está mi razonamiento.

Cada vez que accede a list lo hace sincronizado. Esto es genial. Aunque saque una parte del list en item, no se accede a ese item por varios subprocesos.

Mientras sólo accede a list mientras sincronizada, que debe ser bueno (en su diseño actual.)

Answer 3

Su código tal cual es en hilo de hecho seguro. El razonamiento detrás de esto es dos partes.

El primero es la exclusión mutua. Su sincronización asegura correctamente que solo un hilo a la vez modificará las colecciones.

El segundo tiene que ver con su preocupación por la reorganización del compilador. Usted está preocupado de que la compilación en realidad pueda ordenar la asignación en la que no sería seguro para subprocesos. No tienes que preocuparte por eso en este caso. La sincronización en la lista crea una relación de pasar antes de. Todas las eliminaciones de la lista suceden antes de la escritura en Integer item. Esto le dice al compilador que no puede volver a ordenar la escritura del artículo en ese método.

Answer 4

Su código es seguro para subprocesos, pero no simultáneo (como en paralelo). Como se accede a todo bajo un único bloqueo de exclusión mutua, está serializando todos los accesos; en efecto, el acceso a la estructura es de subproceso único.

Si necesita la funcionalidad tal como se describe en su código de producción, el paquete java.util.concurrent ya proporciona un BlockingQueue con una matriz (tamaño fijo) y implementaciones basadas en una lista enlazada (creíble). Estos son muy interesantes para estudiar ideas de implementación al menos.