Sincronización de campo no final

91

Se muestra una advertencia cada vez que sincronizo en un campo de clase no final. Aquí está el código:

public class X  
{  
   private Object o;  

   public void setO(Object o)  
   {  
     this.o = o;  
   }  

   public void x()  
   {  
     synchronized (o) // synchronization on a non-final field  
     {  
     }  
   }  
 } 

así que cambié la codificación de la siguiente manera:

 public class X  
 {  

   private final Object o;       
   public X()
   {  
     o = new Object();  
   }  

   public void x()  
   {  
     synchronized (o)
     {  
     }  
   }  
 }  

No estoy seguro de que el código anterior sea la forma correcta de sincronizar en un campo de clase no final. ¿Cómo puedo sincronizar un campo no final?

divz
fuente

Respuestas:

127

En primer lugar, le animo a que realmente se esfuerce por tratar los problemas de concurrencia en un nivel superior de abstracción, es decir, resolviéndolo utilizando clases de java.util.concurrent como ExecutorServices, Callables, Futures, etc.

Dicho esto, no hay nada de malo en sincronizar en un campo no final per se. Solo debe tener en cuenta que si la referencia del objeto cambia, la misma sección de código se puede ejecutar en paralelo . Es decir, si un hilo ejecuta el código en el bloque sincronizado y alguien llama setO(...), otro hilo puede ejecutar el mismo bloque sincronizado en la misma instancia al mismo tiempo.

Sincronice el objeto al que necesita acceso exclusivo (o, mejor aún, un objeto dedicado a protegerlo).

aioobe
fuente
1
Estoy diciendo que, si sincroniza en un campo no final, debe tener en cuenta el hecho de que el fragmento de código se ejecuta con acceso exclusivo al objeto al que se ohace referencia en el momento en que se alcanzó el bloque sincronizado. Si el objeto al que se orefiere cambia, puede aparecer otro hilo y ejecutar el bloque de código sincronizado.
aioobe
42
No estoy de acuerdo con su regla general: prefiero sincronizar en un objeto cuyo único propósito es proteger otro estado. Si nunca está haciendo nada con un objeto que no sea bloquearlo, sabe con certeza que ningún otro código puede bloquearlo. Si bloquea un objeto "real" cuyos métodos luego llama, ese objeto también puede sincronizarse consigo mismo, lo que hace que sea más difícil razonar sobre el bloqueo.
Jon Skeet
9
Como digo en mi respuesta, creo que debería justificarme con mucho cuidado por qué querrías hacer tal cosa. Y tampoco recomendaría sincronizar this, recomendaría crear una variable final en la clase únicamente con el propósito de bloquear , lo que evita que cualquier otra persona bloquee el mismo objeto.
Jon Skeet
1
Ese es otro buen punto, y estoy de acuerdo; el bloqueo de una variable no final definitivamente necesita una justificación cuidadosa.
aioobe
No estoy seguro de los problemas de visibilidad de la memoria relacionados con el cambio de un objeto que se utiliza para sincronizar. Creo que te meterías en un gran problema al cambiar un objeto y luego confiar en que el código vea ese cambio correctamente para que "la misma sección de código se pueda ejecutar en paralelo". No estoy seguro de qué garantías, si las hay, extienden el modelo de memoria a la visibilidad de memoria de los campos que se usan para bloquear, a diferencia de las variables a las que se accede dentro del bloque de sincronización. Mi regla general sería que si sincronizas algo, debería ser definitivo.
Mike Q
47

Realmente no es una buena idea, porque sus bloques sincronizados ya no están realmente sincronizados de manera consistente.

Suponiendo que los bloques sincronizados están destinados a garantizar que solo un hilo acceda a algunos datos compartidos a la vez, considere:

  • El hilo 1 entra en el bloque sincronizado. Yay, tiene acceso exclusivo a los datos compartidos ...
  • Hilo 2 llamadas setO ()
  • El hilo 3 (o aún 2 ...) entra en el bloque sincronizado. ¡Eek! Cree que tiene acceso exclusivo a los datos compartidos, pero el hilo 1 todavía está avanzando ...

¿Por qué querrías que esto sucediera? Tal vez haya algunas situaciones muy especializadas en las que tiene sentido ... pero tendrías que presentarme un caso de uso específico (junto con formas de mitigar el tipo de escenario que he dado anteriormente) antes de que esté contento con eso.

Jon Skeet
fuente
2
@aioobe: Pero entonces el hilo 1 aún podría estar ejecutando algún código que está mutando la lista (y al que se refiere con frecuencia o), y en el medio de su ejecución comenzar a mutar una lista diferente. ¿Cómo sería eso una buena idea? Creo que fundamentalmente no estamos de acuerdo sobre si es una buena idea bloquear los objetos que tocas de otras formas. Preferiría poder razonar sobre mi código sin ningún conocimiento de lo que hace otro código en términos de bloqueo.
Jon Skeet
2
@Felype: Parece que debería hacer una pregunta más detallada como una pregunta separada, pero sí, a menudo creo objetos separados como candados.
Jon Skeet
3
@VitBernatik: No. Si el hilo X comienza a modificar la configuración, el hilo Y cambia el valor de la variable que se sincroniza, luego el hilo Z comienza a modificar la configuración, entonces tanto X como Z modificarán la configuración al mismo tiempo, lo cual es malo .
Jon Skeet
1
En resumen, es más seguro si siempre declaramos tales objetos de bloqueo como finales, ¿correcto?
St.Antario
2
@LinkTheProgrammer: "Un método sincronizado sincroniza cada objeto en la instancia" - no, no lo hace. Eso simplemente no es cierto, y debería revisar su comprensión de la sincronización.
Jon Skeet
12

Estoy de acuerdo con uno de los comentarios de John: siempre debe usar un maniquí de bloqueo final al acceder a una variable no final para evitar inconsistencias en caso de que la referencia de la variable cambie. Entonces, en cualquier caso y como primera regla general:

Regla n. ° 1: si un campo no es definitivo, utilice siempre un maniquí de bloqueo final (privado).

Razón # 1: Mantienes el candado y cambias la referencia de la variable tú mismo. Otro hilo que esté esperando fuera del bloqueo sincronizado podrá ingresar al bloque protegido.

Razón # 2: Mantienes el candado y otro hilo cambia la referencia de la variable. El resultado es el mismo: otro hilo puede entrar en el bloque protegido.

Pero cuando se usa un maniquí de bloqueo final, hay otro problema : es posible que obtenga datos incorrectos, porque su objeto no final solo se sincronizará con la RAM cuando llame a sincronizar (objeto). Entonces, como segunda regla empírica:

Regla n. ° 2: Cuando bloquee un objeto no final, siempre debe hacer ambas cosas: Usar un maniquí de bloqueo final y el bloqueo del objeto no final por el bien de la sincronización de RAM. (¡La única alternativa será declarar todos los campos del objeto como volátiles!)

Estos bloqueos también se denominan "bloqueos anidados". Tenga en cuenta que debe llamarlos siempre en el mismo orden, de lo contrario obtendrá un bloqueo :

public class X {
    private final LOCK;
    private Object o;

    public void setO(Object o){
        this.o = o;  
    }  

    public void x() {
        synchronized (LOCK) {
        synchronized(o){
            //do something with o...
        }
        }  
    }  
} 

Como puede ver, escribo los dos candados directamente en la misma línea, porque siempre van juntos. De esta manera, incluso podrías hacer 10 bloqueos de anidación:

synchronized (LOCK1) {
synchronized (LOCK2) {
synchronized (LOCK3) {
synchronized (LOCK4) {
    //entering the locked space
}
}
}
}

Tenga en cuenta que este código no se romperá si adquiere un bloqueo interno como synchronized (LOCK3)en otros hilos. Pero se romperá si llama a otro hilo algo como esto:

synchronized (LOCK4) {
synchronized (LOCK1) {  //dead lock!
synchronized (LOCK3) {
synchronized (LOCK2) {
    //will never enter here...
}
}
}
}

Solo hay una solución alternativa para estos bloqueos anidados al manejar campos no finales:

Regla # 2 - Alternativa: Declare todos los campos del objeto como volátiles. (No hablaré aquí sobre las desventajas de hacer esto, por ejemplo, evitar cualquier almacenamiento en cachés de nivel x incluso para lecturas, etc.)

Por lo tanto, aioobe tiene toda la razón: simplemente use java.util.concurrent. O comience a comprender todo sobre la sincronización y hágalo usted mismo con bloqueos anidados. ;)

Para obtener más detalles sobre por qué se rompe la sincronización en campos no finales, eche un vistazo a mi caso de prueba: https://stackoverflow.com/a/21460055/2012947

Y para obtener más detalles por qué necesita sincronizarse debido a la RAM y las cachés, eche un vistazo aquí: https://stackoverflow.com/a/21409975/2012947

Marcus
fuente
1
Creo que tienes que envolver el setter de ocon un sincronizado (LOCK) para establecer una relación "sucede antes" entre el ajuste y el objeto de lectura o. Estoy discutiendo esto en una pregunta similar mía: stackoverflow.com/questions/32852464/…
Petrakeas
Utilizo dataObject para sincronizar el acceso a los miembros de dataObject. ¿Cómo es eso de malo? Si el objeto de datos comienza a apuntar a algún lugar diferente, quiero que se sincronice con los nuevos datos para evitar que los subprocesos concurrentes lo modifiquen. ¿Algún problema con eso?
Harmen
2

Realmente no veo la respuesta correcta aquí, es decir, está perfectamente bien hacerlo.

Ni siquiera estoy seguro de por qué es una advertencia, no tiene nada de malo. La JVM se asegura de que recupere algún objeto válido (o nulo) cuando lea un valor, y que pueda sincronizar en cualquier objeto.

Si planea cambiar el bloqueo mientras está en uso (en lugar de, por ejemplo, cambiarlo desde un método de inicio, antes de comenzar a usarlo), debe crear la variable que planea cambiar volatile. Entonces, todo lo que necesita hacer es sincronizar tanto el objeto antiguo como el nuevo, y puede cambiar el valor de forma segura

public volatile Object lock;

...

synchronized (lock) {
    synchronized (newObject) {
        lock = newObject;
    }
}

Ahí. No es complicado, escribir código con bloqueos (mutex) es realmente bastante fácil. Escribir código sin ellos (código libre de bloqueo) es lo difícil.

Evan Dark
fuente
Puede que esto no funcione. Digamos que o comenzó como referencia a O1, luego el hilo T1 bloquea o (= O1) y O2 y establece o en O2. Al mismo tiempo, Thread T2 bloquea O1 y espera a que T1 lo desbloquee. Cuando reciba el bloqueo O1, se pondrá o en O3. En este escenario, entre T1 liberando O1 y T2 bloqueando O1, O1 se volvió inválido para bloquear a través de o. En este momento, otro hilo puede usar o (= O2) para bloquear y continuar ininterrumpidamente en carrera con T2.
GPS
2

EDITAR: Entonces, esta solución (como sugirió Jon Skeet) podría tener un problema con la atomicidad de la implementación de "sincronizado (objeto) {}" mientras que la referencia del objeto está cambiando. Pregunté por separado y, según el Sr. Erickson, no es seguro para subprocesos; consulte: ¿Entrar en bloque sincronizado es atómico? . Así que tómalo como ejemplo de cómo NO hacerlo, con enlaces por qué;)

Vea el código cómo funcionaría si synchronized () fuera atómico:

public class Main {
    static class Config{
        char a='0';
        char b='0';
        public void log(){
            synchronized(this){
                System.out.println(""+a+","+b);
            }
        }
    }

    static Config cfg = new Config();

    static class Doer extends Thread {
        char id;

        Doer(char id) {
            this.id = id;
        }

        public void mySleep(long ms){
            try{Thread.sleep(ms);}catch(Exception ex){ex.printStackTrace();}
        }

        public void run() {
            System.out.println("Doer "+id+" beg");
            if(id == 'X'){
                synchronized (cfg){
                    cfg.a=id;
                    mySleep(1000);
                    // do not forget to put synchronize(cfg) over setting new cfg - otherwise following will happend
                    // here it would be modifying different cfg (cos Y will change it).
                    // Another problem would be that new cfg would be in parallel modified by Z cos synchronized is applied on new object
                    cfg.b=id;
                }
            }
            if(id == 'Y'){
                mySleep(333);
                synchronized(cfg) // comment this and you will see inconsistency in log - if you keep it I think all is ok
                {
                    cfg = new Config();  // introduce new configuration
                    // be aware - don't expect here to be synchronized on new cfg!
                    // Z might already get a lock
                }
            }
            if(id == 'Z'){
                mySleep(666);
                synchronized (cfg){
                    cfg.a=id;
                    mySleep(100);
                    cfg.b=id;
                }
            }
            System.out.println("Doer "+id+" end");
            cfg.log();
        }
    }

    public static void main(String[] args) throws InterruptedException {
        Doer X = new Doer('X');
        Doer Y = new Doer('Y');
        Doer Z = new Doer('Z');
        X.start();
        Y.start();
        Z.start();
    }

}
Vit Bernatik
fuente
1
Esto podría estar bien, pero no sé si hay alguna garantía en el modelo de memoria de que el valor en el que sincroniza sea el escrito más recientemente. No creo que haya ninguna garantía de "lectura y sincronización" atómica. Personalmente trato de evitar la sincronización en monitores que tienen otros usos de todos modos, por simplicidad. (Al tener un campo separado, el código se vuelve claramente correcto en lugar de tener que razonar al respecto con cuidado).
Jon Skeet
@Jon. ¡Gracias por la respuesta! Escucho tu preocupación. Estoy de acuerdo para este caso que el bloqueo externo evitaría la cuestión de la "atomicidad sincronizada". Por lo tanto, sería preferible. Aunque puede haber casos en los que desee introducir en tiempo de ejecución más configuraciones y compartir configuraciones diferentes para diferentes grupos de subprocesos (aunque no es mi caso). Y entonces esta solución podría volverse interesante. Publiqué la pregunta stackoverflow.com/questions/29217266/… de atomicidad sincronizada (), así que veremos si se puede usar (y alguien responde)
Vit Bernatik
2

AtomicReference se adapta a sus necesidades.

De la documentación de java sobre el paquete atómico :

Un pequeño conjunto de herramientas de clases que admiten programación segura para subprocesos sin bloqueo en variables individuales. En esencia, las clases en este paquete extienden la noción de valores volátiles, campos y elementos de matriz a aquellos que también proporcionan una operación de actualización condicional atómica del formulario:

boolean compareAndSet(expectedValue, updateValue);

Código de muestra:

String initialReference = "value 1";

AtomicReference<String> someRef =
    new AtomicReference<String>(initialReference);

String newReference = "value 2";
boolean exchanged = someRef.compareAndSet(initialReference, newReference);
System.out.println("exchanged: " + exchanged);

En el ejemplo anterior, reemplaza Stringcon el suyoObject

Pregunta SE relacionada:

¿Cuándo usar AtomicReference en Java?

Ravindra babu
fuente
1

Si onunca cambia durante la vida útil de una instancia de X, la segunda versión tiene mejor estilo independientemente de si se trata de sincronización.

Ahora bien, es imposible responder si hay algún problema con la primera versión sin saber qué más está sucediendo en esa clase. Yo tendería a estar de acuerdo con el compilador en que parece propenso a errores (no repetiré lo que han dicho los demás).

NPE
fuente
1

Solo agrego mis dos centavos: tuve esta advertencia cuando usé un componente que se instancia a través del diseñador, por lo que su campo no puede ser final, porque el constructor no puede tomar parámetros. En otras palabras, tenía un campo cuasifinal sin la palabra clave final.

Creo que por eso es solo una advertencia: probablemente estás haciendo algo mal, pero también podría estar bien.

maní
fuente