2011-02-02 11 views
6

Hice un objeto Stack sincronizado simple en Java, solo para fines de capacitación. Esto es lo que hice:¿Cómo crear correctamente una clase SynchronizedStack?

public class SynchronizedStack { 
    private ArrayDeque<Integer> stack; 

    public SynchronizedStack(){ 
     this.stack = new ArrayDeque<Integer>();  
    } 

    public synchronized Integer pop(){ 
     return this.stack.pop(); 
    } 

    public synchronized int forcePop(){ 
     while(isEmpty()){ 
      System.out.println(" Stack is empty"); 
      try { 
       wait(); 
      } catch (InterruptedException e) { 
       e.printStackTrace(); 
      } 
     } 
     return this.stack.pop(); 
    } 

    public synchronized void push(int i){ 
     this.stack.push(i); 
     notifyAll(); 
    } 

    public boolean isEmpty(){ 
     return this.stack.isEmpty(); 
    } 

    public synchronized void pushAll(int[] d){ 
     for(int i = 0; i < d.length; i++){ 
      this.stack.push(i); 
     } 
     notifyAll(); 
    } 

    public synchronized String toString(){ 
     String s = "["; 
     Iterator<Integer> it = this.stack.iterator(); 
     while(it.hasNext()){ 
      s += it.next() + ", "; 
     } 
     s += "]"; 
     return s; 
    } 
} 

Aquí están mis preguntas:

  • ¿Está bien que no sincronice el método isEmtpy()? Pensé que era porque incluso si otro hilo está modificando la pila al mismo tiempo, aún así devolvería un resultado coherente (no hay ninguna operación que entre en un estado isEmpty que no sea ni inicial ni final). ¿O es un mejor diseño tener sincronizados todos los métodos de un objeto sincronizado?

  • No me gusta el método forcePop(). Solo quería crear un hilo que pudiera esperar hasta que se insertara un elemento en la pila antes de mostrar un elemento, y pensé que la mejor opción era hacer el ciclo con el wait() en el método run() del hilo, pero puedo 't porque arroja un IllegalMonitorStatException. ¿Cuál es el método adecuado para hacer algo como esto?

  • ¿Algún otro comentario/sugerencia?

¡Gracias!

+1

No bloquee los métodos, prefiera bloquear los objetos. Lea esto: http://download.oracle.com/javase/tutorial/essential/concurrency/locksync.html –

+0

Stack extends Vector, que ya está sincronizado. Una elección diferente de la colección podría ser mejor para este ejercicio de entrenamiento. –

+0

Don Roby: Sí, como lo señala axtavt, ahora estoy usando ArrayDeque. – nbarraille

Respuesta

7
  • Stack sí ya está sincronizado, por lo que no tiene sentido aplicar la sincronización de nuevo (utilizar ArrayDeque si quieres una aplicación de pila no sincronizada)

  • NO ES CORRECTO (aparte del hecho desde el punto anterior), porque la falta de sincronización puede causar efectos de visibilidad en la memoria.

  • forcePop() es bastante bueno.Aunque debe pasar InterruptedException sin atraparlo para seguir el contrato del método de bloqueo interrumpible. Le permitiría interrumpir un hilo bloqueado en la llamada forcePop() llamando al Thread.interrupt().

+0

Supongamos que he codificado esto sin depender de la implementación Stack/ArrayDeque, pero solo con las matrices de una manera que estoy seguro de que no hay un estado transitivo que pueda causar resultados incoherentes isEmpty(). ¿Sería mejor no sincronizarlo (más rápido) o sincronizarlo de todos modos (más seguro)? – nbarraille

+0

Sincronícelo de todos modos: el problema es que, sin alguna sincronización, es posible que nunca vea las actualizaciones de otro hilo en su llamada isEmpty(). – Sbodd

0

El único problema con la falta de sincronización de isEmpty() es que no se sabe lo que sucede debajo. Si bien su razonamiento es, bueno, razonable, asume que el subyacente Stack también se comporta de manera razonable. Lo que probablemente sea en este caso, pero no puede confiar en ello en general.

Y la segunda parte de su pregunta, no hay nada de malo con una operación pop bloqueante, vea this para una implementación completa de todas las estrategias posibles.

Y otra sugerencia: si está creando una clase que probablemente se reutilice en varias partes de una aplicación (o incluso varias), no use los métodos synchronized. Hacer esto en su lugar:

public class Whatever { 
    private Object lock = new Object(); 

    public void doSomething() { 
    synchronized(lock) { 
     ... 
    } 
    } 
} 

La razón de esto es que no se sabe muy bien si los usuarios de la clase desea sincronizar en sus Whatever instancias o no. Si lo hacen, podrían interferir con la operación de la clase en sí. De esta manera tienes tu propio candado privado con el que nadie puede interferir.

0

Suponiendo que stack.isEmpty() no necesite la sincronización podría ser cierto, pero está confiando en un detalle de implementación de una clase sobre la que no tiene control. Los javadocs de Stack indican que la clase no es segura para subprocesos, por lo que debe sincronizar todos los accesos.

0

Creo que estás mezclando expresiones idiomáticas un poco. Está respaldando su SynchronizedStack con java.util.Stack, que a su vez está respaldado por java.util.Vector, que es synchronized. Creo que debería encapsular el comportamiento wait() y notify() en otra clase.

Cuestiones relacionadas