2009-10-27 7 views
10

Estoy tratando de crear una implementación de cola libre de bloqueo en Java, principalmente para el aprendizaje personal. La cola debe ser general, permitiendo cualquier número de lectores y/o escritores al mismo tiempo.¿Es esto (bloqueo) la implementación de cola Thread-Safe?

¿Podría revisarlo y sugerir cualquier mejora o problema que encuentre?

Gracias.

import java.util.concurrent.atomic.AtomicReference; 

public class LockFreeQueue<T> { 
    private static class Node<E> { 
     E value; 
     volatile Node<E> next; 

     Node(E value) { 
      this.value = value; 
     } 
    } 

    private AtomicReference<Node<T>> head, tail; 

    public LockFreeQueue() { 
     // have both head and tail point to a dummy node 
     Node<T> dummyNode = new Node<T>(null); 
     head = new AtomicReference<Node<T>>(dummyNode); 
     tail = new AtomicReference<Node<T>>(dummyNode); 
    } 

    /** 
    * Puts an object at the end of the queue. 
    */ 
    public void putObject(T value) { 
     Node<T> newNode = new Node<T>(value); 
     Node<T> prevTailNode = tail.getAndSet(newNode); 
     prevTailNode.next = newNode; 
    } 

    /** 
    * Gets an object from the beginning of the queue. The object is removed 
    * from the queue. If there are no objects in the queue, returns null. 
    */ 
    public T getObject() { 
     Node<T> headNode, valueNode; 

     // move head node to the next node using atomic semantics 
     // as long as next node is not null 
     do { 
      headNode = head.get(); 
      valueNode = headNode.next; 
      // try until the whole loop executes pseudo-atomically 
      // (i.e. unaffected by modifications done by other threads) 
     } while (valueNode != null && !head.compareAndSet(headNode, valueNode)); 

     T value = (valueNode != null ? valueNode.value : null); 

     // release the value pointed to by head, keeping the head node dummy 
     if (valueNode != null) 
      valueNode.value = null; 

     return value; 
} 
+0

portado a revisión de código en http://codereview.stackexchange.com/questions/224/is-this-lock-free-queue-implementation-thread-safe –

Respuesta

4

El código no es seguro para subprocesos. Consideremos putObject(...):

public void putObject(T value) { 
    Node<T> newNode = new Node<T>(value); 
    Node<T> prevTailNode = tail.getAndSet(newNode); 
    prevTailNode.next = newNode; 
} 

La segunda sentencia añade el nuevo nodo antes puntero del nodo anterior next se ha establecido. Eso solo sucede en la tercera declaración. Por lo tanto, hay una ventana en la que next es null; es decir, una condición de carrera.

Incluso si lo solucionó, hay un problema más insidioso. Un hilo que lea el campo next para un objeto Nodo no será necesariamente para ver el valor que acaba de escribir un segundo hilo. Esa es una consecuencia del modelo de memoria de Java. En este caso, la forma de asegurar que la siguiente lectura siempre ve el valor escrito anteriormente es o bien:

  • declaran next ser volatile, o
  • hacer tanto la lectura como la escritura en un mutex primitiva en el mismo objeto

EDIT: en la lectura del código de getObject() y putObject() con más detalle, puedo ver que nada obliga al valor no nulo de next a ser enviado a la memoria en putObject, y las fuerzas nada getObject leer next de la principal memoria. Por lo tanto, el código getObject podría ver el valor incorrecto de next, haciendo que devuelva null cuando realmente hay un elemento en la cola.

+0

Disculpa, pero te equivocas al ser nulo el siguiente. En realidad, tail.next siempre es nulo y no crea ningún problema. Si el siguiente es nulo en getObject, el ciclo termina y se devuelve null (la cola está vacía). Además, el ciclo en getObject puede girar solo si otro hilo ha escrito correctamente el encabezado, por ej. el otro subproceso tuvo éxito, que es lo que queremos de los algoritmos sin bloqueo. – jpalecek

+0

@jpalecek - arreglado. –

+0

Lo sentimos, pero sigue siendo incorrecto, en muchas áreas (el EDIT2 completo, por ejemplo, indica que no ha notado que la cola siempre contiene al menos un nodo ficticio). – jpalecek

1

veo sólo dos problemas con su código:

  • uno es el problema con las operaciones de memoria ordenar Stephen C mencionado (puede ser resuelto por la que se declara next y valuevolatile) (Nodo: valor tiene el mismo problema)

  • segundo es uno más sutil, y no relacionado con la concurrencia: después de devolver un objeto en getObject, aún conservas su referencia de la cabecera. Esto podría provocar una pérdida de memoria.

De lo contrario, el algoritmo está bien. Una demostración vaga (suponga que lo anterior está solucionado):

L1: tail nunca se puede eliminar de la cola. Esto se cumple porque cuando algo se almacena en tail, tiene next == null.Además, cuando asigna algo a xxx.next (solo en putObject), no puede ser tail, debido a la atomicidad de getAndSet y al orden entre la escritura volátil y la posterior lectura, suponiendo que lee un tail.next no nulo, este valor debe haber sido escrito por putObject y por lo tanto sucede después de la última línea de este. Esto significa que sucede después de la línea anterior, lo que significa que el valor que estamos leyendo no es de tail.

Como consecuencia, cada objeto en putObject será accesible desde head. Esto se debe a que nos estamos conectando después de tail, y este nodo se puede eliminar solo después de escribir la referencia del nuevo nodo a su next, lo que significa que se puede acceder al nuevo nodo desde head.

Los objetos añadidos se ordenan trivialmente mediante la operación getAndSet en putObject.

Los objetos eliminados se ordenan según las operaciones exitosas compareAndSet en getObject.

getObject/putObject se ordenan de acuerdo con la escritura/lectura del campo volátil next.

+0

¿Qué quiere decir con "todavía conservas su referencia de la cabeza"? 'head' está establecido en' nextNode' en ese método. – Joren

+1

Quiero decir, el 'valor 'que estaba en' nextNode' se devuelve y se conserva a través de 'head'. Si no se toman más elementos de la cola (como, por ejemplo, si la cola está vacía), la referencia al valor devuelto nunca se libera. – jpalecek

+0

¡Gracias! He solucionado el problema de pérdida de memoria y volví 'next' volátil. Pero aún no entiendo por qué 'value' necesita ser volátil. ¿Podrías explicar más? –

1

creo que obtendría un NPE cuando intenta "liberar el valor ..." si se llama

new LockFreeQueue<?>().getObject(); 

ya que no se realiza ninguna comprobación de nulidad en valueNode allí, a pesar de la protección contra arriba

+0

Tienes razón. ¡Me había perdido eso totalmente! Lo arreglé ahora. ¡Gracias! –

1

Usted debe echar un vistazo a la implementación de java.util.concurrent.ConcurrentLinkedQueue http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ConcurrentLinkedQueue.html Esto lo hace bastante más de lo que está tratando de lograr

+0

Gracias. Lo verificaré para obtener más ideas. Sin embargo, el 'ConcurrentLinkedQueue' es demasiado complejo, ya que admite muchos métodos, mientras que mi cola es mucho más simple, lo que me permite hacer más suposiciones y probar más optimizaciones. –

+1

Existe un argumento de que el código de bloqueo es complejo por naturaleza. ;) –

Cuestiones relacionadas