2009-10-28 36 views
10

Tengo la siguiente clase que contiene un solo campo i. El acceso a este campo está protegido por el bloqueo del objeto ("esto"). Al implementar equals() necesito bloquear esta instancia (a) y la otra (b). Si el hilo 1 llama a.equals (b) y al mismo tiempo el hilo 2 llama a b.equals (a), el orden de bloqueo es inverso en las dos implementaciones y puede dar como resultado un interbloqueo.Sincronización correcta equals() en Java

¿Cómo debo implementar equals() para una clase que tiene campos sincronizados?

public class Sync { 
    // @GuardedBy("this") 
    private int i = 0; 
    public synchronized int getI() {return i;} 
    public synchronized void setI(int i) {this.i = i;} 

    public int hashCode() { 
     final int prime = 31; 
     int result = 1; 
     synchronized (this) { 
      result = prime * result + i; 
     } 
     return result; 
    } 

    public boolean equals(Object obj) { 
     if (this == obj) 
      return true; 
     if (obj == null) 
      return false; 
     if (getClass() != obj.getClass()) 
      return false; 
     Sync other = (Sync) obj; 
     synchronized (this) { 
      synchronized (other) { 
       // May deadlock if "other" calls 
       // equals() on "this" at the same 
       // time 
       if (i != other.i) 
        return false; 
      } 
     } 
     return true; 
    } 
} 

Respuesta

0

La correcta aplicación de equals() y hashCode() es requerida por varias cosas como hash estructuras de datos, y por lo que no tienen otra opción real del país. Desde otra perspectiva, equals() y hashCode() son solo métodos, con los mismos requisitos de sincronización que otros métodos. Todavía tiene el problema de interbloqueo, pero no es específico del hecho de que equals() lo está causando.

7

¿Por qué sincronizar? Cuál es el caso de uso en el que importa uno si cambia durante la comparación y no importa si se produce un cambio inmediatamente después de que se ejecute el código en función de la igualdad. (es decir, si tiene un código que depende de la eficiencia, ¿qué ocurre si los valores se vuelven desiguales antes o durante este código)?

Creo que tiene que echar un vistazo al proceso para ver dónde debe bloquear.

+0

Tiene razón en que un valor podría cambiar inmediatamente después de la comparación, pero como cuando el valor envuelto es más interesante que un 'int' primitivo, existe la posibilidad de que ese valor esté en un estado incoherente a menos que se sincronice mientras se examinan sus partes. –

+1

Es cierto, pero todavía tiene el problema de qué hacer después de que el igual – Mark

+2

en realidad, el problema es de visibilidad. sin usar sincronizado, un pedido válido aún podría dar como resultado un resultado inesperado (es decir, un valor establecido anteriormente no es visible cuando debería). – jtahlborn

1

Siempre encerrarlos en el mismo orden, de una manera que podría decidir el orden es sobre los resultados de System.identityHashCode(Object)

Editar para incluir comentario:

La mejor solución para hacer frente a los raros casos de el hecho de que identityHashCodes sea igual requiere más detalles sobre qué otro bloqueo de esos objetos está sucediendo.

Todos los requisitos de bloqueo de objetos múltiples deben usar el mismo proceso de resolución.

Puede crear una utilidad compartida para rastrear objetos con el mismo identificadorHashCode durante el corto período de los requisitos de bloqueo, y proporcionar un orden repetible para ellos durante el período de tiempo en que se los rastrea.

+0

public boolean equals (Object obj) { if (this == obj) return true; if (obj == null) return false; if (getClass()! = Obj.getClass()) return false; Sincronizar other = (Sync) obj; booleano smallerHash = System.identityHashCode (this) Philipp

+0

'System.identityHashCode' no da valores únicos !! –

9

Intentar sincronizar equals y hashCode dentro del objeto no funcionará correctamente. Considere el caso de un HashMap que usa hashCode para descubrir en qué "cubo" estará un objeto, y luego usa equals para buscar secuencialmente todos los objetos en el cubo.

Si se permite que los objetos a mutar en una forma que cambia los resultados de hashCode o equals que podría terminar con un escenario en el que HashMap llamadas hashCode. Adquiere el bloqueo, obtiene el hash y libera el bloqueo nuevamente. HashMap luego procede a calcular qué "cubo" usar. Pero antes de que HashMap pueda adquirir el bloqueo en iguales, alguien más toma el bloqueo y muta el objeto para que equals se vuelva incoherente con el valor anterior de hashCode. Esto conducirá a resultados catastróficos.

Los métodos hashCode y equals se utilizan en muchos lugares y son fundamentales para la API de colecciones de Java. Podría ser valioso repensar la estructura de su aplicación que no requiere acceso sincronizado a estos métodos.O al menos no sincronizar en el objeto mismo.

+0

¿Por qué el voto a favor? Si miras las clases en el JDK (java.util.Date o java.lang.Long), encontrarás que tampoco están sincronizadas. – leonm

+1

"Si se permite que los objetos muten de una manera que cambie los resultados de hashCode o sea igual a ..." La mayoría de los objetos lo hacen, incluso java.util.Date (setTime (long)). Siempre es necesario tener esto en cuenta al usar una estructura de datos hash; no la cambie después de haberla procesado. Aún así, está permitido cambiarlo. – sfussenegger

6

¿Dónde está el punto de sincronización de igual a igual() si el resultado no está garantizado para ser verdad después de la sincronización se dejó:

if (o1.equals(o2)) { 
    // is o1 still equal to o2? 
} 

Por lo tanto usted podría simplemente sincronizar las llamadas a Geti() es igual a uno tras otro en el interior sin cambiar la salida, ya no es válido.

Usted siempre tiene que sincronizar todo el bloque:

synchronized(o1) { 
    synchronized(o2) { 
    if (o1.equals(o2)) { 
     // is o1 still equal to o2? 
    } 
    } 
} 

Es cierto que usted todavía se enfrentan al mismo problema, pero al menos su sincronización en el punto correcto;)

1

El único La forma de saber con certeza si la sincronización es estrictamente necesaria es analizar el programa completo para situaciones. Hay dos cosas que debes buscar; situaciones donde un hilo está cambiando un objeto mientras otro llama a iguales, y situaciones donde el hilo que llama al equals puede ver un valor obsoleto de i.

Si bloquea ambos this y el objeto other al mismo tiempo, de hecho corre el riesgo de un punto muerto. Pero cuestionaría que necesites hacer esto. En lugar de ello, creo que se debe implementar equals(Object) así:

public boolean equals(Object obj) { 
    if (this == obj) 
     return true; 
    if (obj == null) 
     return false; 
    if (getClass() != obj.getClass()) 
     return false; 
    Sync other = (Sync) obj; 
    return this.getI() == other.getI(); 
} 

esto no garantiza que los dos objetos tienen el mismo valor de i al mismo tiempo, pero que es poco probable que cualquier diferencia práctica. Después de todo, incluso si tuviera esa garantía, igual tendría que lidiar con el problema de que los dos objetos ya no podrían ser iguales en el momento en que devolvió la llamada equals. (¡Este es el punto de @s!)

Además, esto no elimina por completo el riesgo de interbloqueo. Considere el caso en que un hilo puede llamar al equals mientras mantiene un bloqueo en uno de los dos objetos; p.ej.

// In same class as above ... 
public synchronized void frobbitt(Object other) { 
    if (this.equals(other)) { 
     ... 
    } 
} 

Ahora bien, si dos subprocesos llaman a.frobbitt(b) y b.frobbitt(a) respectivamente, existe el riesgo de estancamiento.

(Sin embargo, sí es necesario para llamar getI() o declarar i ser volatile, de lo contrario el equals() pudo ver un valor rancio de i si se ha actualizado recientemente de un hilo diferente.)

Dicho esto, hay algo bastante preocupante acerca de un método basado en el valor equals en un objeto cuyos valores de componentes pueden estar mutados. Por ejemplo, esto romperá muchos de los tipos de colección. Combina esto con multi-threading y vas a tener muchas dificultades para averiguar si tu código es realmente correcto. No puedo dejar de pensar que sería mejor cambiar los métodos equals y hashcode para que no dependan del estado que puede mutar después de haber llamado los métodos por primera vez.

+0

> Además, esto no elimina por completo el riesgo de interbloqueo. Considere el caso en que un hilo puede llamar igual mientras mantiene un bloqueo en uno de los dos objetos. No estoy seguro de cómo esto es un problema. Si es el bloqueo de esto, ningún problema como getI() se reentrará y no afectará a other.getI(). Lo mismo si es la cerradura del otro. Solo como referencia sobre cómo lo hacen los demás: Vector se sincroniza en este pero no en el otro en igual(). Esto puede llevar a ConcurrentModificationException si el vector se modifica mientras se ejecuta equals(). AtomicInteger no implementa equals() – Philipp

+0

@Philipp: necesitaría dos subprocesos haciendo esto con el mismo par de objetos en posiciones opuestas para obtener un interbloqueo. –

+0

¿El this.getI() no liberará su bloqueo antes de llamar al otro.getI()? No entiendo cómo esto podría empantanar. – Philipp

1

(supongo que usted está interesado en el caso general aquí, y no sólo en números enteros envueltos.)

No se puede evitar que dos hilos de llamar ... set métodos en orden arbitrario. Entonces, incluso cuando un hilo obtiene un (válido) true llamando al .equals( ...), ese resultado podría ser invalidado inmediatamente por otro hilo que llame al set ... en uno de los objetos. IOW el resultado solo significa que los valores fueron iguales en el instante de comparación.

Por lo tanto, la sincronización protegería contra la caja del valor envuelta estar en un estado incoherente, mientras que usted está tratando de hacer las comparar (por ejemplo dos mitades int -sized de una envuelta long se actualizan de forma consecutiva). Puede evitar una condición de carrera copiando cada valor (es decir, independientemente sincronizado, sin superposición) y luego comparando las copias.

-2

Lee y escribe en int las variables ya son atómicas, por lo que no es necesario sincronizar el captador y el colocador (consulte http://java.sun.com/docs/books/tutorial/essential/concurrency/atomic.html).

Del mismo modo, no necesita sincronizar equals aquí. Aunque podría evitar que otro subproceso cambie uno de los valores i durante la comparación, ese subproceso simplemente se bloqueará hasta que se complete el método equals y lo cambie inmediatamente después.

0

Como señala Jason Day, las comparaciones enteras ya son atómicas, por lo que la sincronización aquí es superflua. Pero si solo estuviera construyendo un ejemplo simplificado y en la vida real está pensando en un objeto más complejo:

La respuesta directa a su pregunta es, asegúrese de que siempre compare los artículos en un orden consistente. No importa cuál sea ese orden, siempre y cuando sea consistente. En un caso como este, System.identifyHashCode proporcionaría un ordenamiento, como:

public boolean equals(Object o) 
{ 
    if (this==o) 
    return true; 
    if (o==null || !o instanceof Sync) 
    return false; 
    Sync so=(Sync) o; 
    if (System.identityHashCode(this)<System.identityHashCode(o)) 
    { 
    synchronized (this) 
    { 
     synchronized (o) 
     { 
     return equalsHelper(o); 
     } 
    } 
    } 
    else 
    { 
    synchronized (o) 
    { 
     synchronized (this) 
     { 
     return equalsHelper(o); 
     } 
    } 
    } 
} 

Entonces declarar equalsHelper privada y deja que haga el trabajo real de la comparación.

(Pero wow, eso es un montón de código para un tema tan trivial.)

Tenga en cuenta que para que esto funcione, cualquier función que puede cambiar el estado del objeto tendría que ser declarada sincronizada.

Otra opción sería sincronizar en Sync.class en lugar de en cualquier objeto, y luego también sincronizar cualquier setter en Sync.class. Esto bloquearía todo en un único mutex y evitaría todo el problema. Por supuesto, dependiendo de lo que esté haciendo esto podría causar un bloqueo no deseado de algunos hilos. Tendría que pensar en las implicaciones a la luz de lo que trata su programa.

Si esto es un problema real en un proyecto en el que está trabajando, una alternativa seria a considerar sería hacer que el objeto sea inmutable. Piensa en String y StringBuilder. Puede crear un objeto SyncBuilder que le permita realizar cualquier trabajo que necesite para crear una de estas cosas, luego tener un objeto Sync cuyo estado haya sido establecido por el constructor y nunca podrá cambiar. Cree un constructor que tome un SyncBuilder y establezca su estado para que coincida o tenga un método SyncBuilder.toSync. De cualquier manera, usted hace todo su edificio en SyncBuilder, luego lo convierte en Sincronización y ahora tiene garantizada la inmutabilidad para que no tenga que meterse con la sincronización en absoluto.

3

Si se ha dicho lo suficiente, los campos que utiliza para hashCode(), equals() o compareTo() deben ser inmutables, preferiblemente finales. En este caso, no necesita sincronizarlos.

La única razón para implementar hashCode() es para que el objeto se pueda agregar a una colección hash, y no se puede cambiar válidamente el hashCode() de un objeto que se ha agregado a dicha colección.

+1

¿Le gustan los cálculos para ArrayList? ;-) Bromas aparte, la final inmutable es algo bueno por lo que luchar (probablemente más allá de lo bueno, todo es * tan * mucho más fácil cuando usas objetos inmutables), pero no siempre es posible con objetos compuestos grandes. –

+0

¿Cómo se usa hashCode() con ArrayList? Si bien puede que no sea posible hacer que los objetos sean inmutables, no cambia el hecho de que no puede cambiar el código hash() de una tecla de un mapa o elementos de un conjunto y hacer que funcione. –

0

No use sincronizaciones. Piensa en los granos no modificables.

2

Está intentando definir un contenido "basado en" igual "y" hashCode "en un objeto mutable. Esto no solo es imposible: no tiene sentido. De acuerdo con

http://java.sun.com/javase/6/docs/api/java/lang/Object.html

tanto "iguales" y "código hash" tienen que ser coherentes: devolver el mismo valor para las invocaciones sucesivas sobre el mismo objeto (s). La mutabilidad por definición impide eso. Esto no es solo teoría: muchas otras clases (por ejemplo, colecciones) dependen de los objetos que implementan la semántica correcta para equals/hashCode.

El problema de la sincronización es una pista falsa aquí. Cuando resuelva el problema subyacente (mutabilidad), no necesitará sincronizar. Si no resuelve el problema de mutabilidad, ninguna cantidad de sincronización lo ayudará.

+3

Los objetos mutables de la JVM implementan iguales (ejemplo: ArrayList, HashMap) por lo que su declaración de que "basado en contenido" es igual a "(...) en un objeto mutable (...) no tiene sentido" no se comparte por aquellos que escriben la JVM. Incluso algunas clases mutable sychronized implementan iguales (ejemplo Vector, que no es seguro deadlock por cierto)! – Philipp

+1

Ese es un buen punto. Sin embargo, yo diría que es responsabilidad del que llama hacer estos objetos efectivamente inmutables durante la duración de cualquier operación que haga uso de equals/hashCode. Por ejemplo, llamar a List .contains (x) mientras muta x, o mutar las claves en Map Tiene un comportamiento indefinido. – user58804

0

Debe asegurarse de que los objetos no cambian entre las llamadas a hashCode() y equals() (si se llama). Luego debe asegurarse de que los objetos no cambien (en la medida en que hashCode e iguales se refieren) mientras el objeto se encuentra en un hashmap. Para cambiar el objeto, primero debe eliminarlo, luego cámbielo y vuelva a colocarlo.

0

Como han mencionado otros, si las cosas cambian durante la comprobación de igual, ya existe la posibilidad de un comportamiento loco (incluso con la sincronización correcta). entonces, todo lo que realmente necesita preocuparse es la visibilidad (quiere asegurarse de que un cambio que "pase antes" sea visible su llamada igual). Por lo tanto, sólo se puede hacer "instantánea" es igual que será correcta en términos de "ocurre antes de" relación y no va a sufrir de problemas de orden de bloqueos:

public boolean equals(Object o) { 
    // ... standard boilerplate here ... 

    // take a "snapshot" (acquire and release each lock in turn) 
    int myI = getI(); 
    int otherI = ((Sync)o).getI(); 

    // and compare (no locks held at this point) 
    return myI == otherI; 
}