java - español - pmd

Inicialización perezosa incorrecta

Question

Findbug me dijo que uso una inicialización perezosa incorrecta.

public static Object getInstance() {
if (instance != null) {
return instance;
}
instance = new Object();
return instance;
}

No veo nada malo aquí. ¿Es el comportamiento equivocado de findbug, o me he perdido algo?

Answer 1

Desde 1.5: la instancia debe ser volátil y debe integrar una variable tmp para evitar el uso de una instancia que se crea, pero su inicialización aún no está terminada.

private static volatile Object myLock = new Object();
private static volatile Object instance;
public static Object getInstance() {
if (instance == null) {
Object tmpObj;
synchronized (myLock) {
tmpObj = instance;
if (tmpObj == null) {
tmpObj = new Object();
}
}
instance = tmpObj;
}
return instance;
}

Answer 2

Findbug está haciendo referencia a un posible problema de subprocesos. En un entorno de subprocesos múltiples, existe la posibilidad de que su singleton se cree más de una vez con su código actual.

Hay mucha lectura here , pero ayudará a explicar.

La condición de la carrera aquí está en el if check . En la primera llamada, un subproceso entrará en el if check , y creará la instancia y la asignará a "instancia". Pero existe la posibilidad de que otro subproceso se active entre la if check y la creación / asignación de la instancia. Este hilo también podría pasar la if check porque la asignación aún no se ha realizado. Por lo tanto, se crearían dos (o más, si se incorporaran más subprocesos) instancias, y sus subprocesos tendrían referencias a diferentes objetos.

Answer 3

Gracias a John Klehm por la muestra publicada.

También puede intentar asignar la instancia del objeto en el bloque sincronizado directamente

synchronized (MyCurrentClass.myLock=new Object())

es decir

private static volatile Object myLock = new Object();
public static Object getInstance() {
if (instance == null) { // avoid sync penalty if we can
synchronized (MyCurrentClass.myLock**=new Object()**) { // declare a private static Object to use for mutex
if (instance == null) { // have to do this inside the sync
instance = new Object();
}
}
}
return instance;
}

Answer 4

Necesita hacer un bloqueo alrededor de la instanciación para hacer esto correcto

LI: Inicialización perezosa incorrecta del campo estático (LI_LAZY_INIT_STATIC)

Este método contiene una inicialización perezosa no sincronizada de un campo estático no volátil. Debido a que el compilador o procesador puede reordenar las instrucciones, no se garantiza que los subprocesos vean un objeto completamente inicializado, si el subproceso puede ser llamado por múltiples subprocesos. Puedes hacer que el campo sea volátil para corregir el problema. Para obtener más información, consulte el sitio web de Java Memory Model.

Answer 5

Su código es un poco más complejo de lo necesario, por lo que podría confundirse.

Edición: Definitivamente es el tema de los subprocesos que publicaron los demás, pero pensé que publicaría la implementación de la verificación de doble bloqueo aquí como referencia a continuación:

private static final Object lock = new Object();
private static volatile Object instance; // must be declared volatile
public static Object getInstance() {
if (instance == null) { // avoid sync penalty if we can
synchronized (lock) { // declare a private static Object to use for mutex
if (instance == null) { // have to do this inside the sync
instance = new Object();
}
}
}
return instance;
}

Answer 6

Te perdiste el problema de subprocesos múltiples,

private static Object instance;
public static synchronized Object getInstance() {
return (instance != null ? instance : (instance = new Object()));
}

Answer 7

su objeto estático no está sincronizado Además su método no es una inicialización perezosa. Normalmente, lo que hace es mantener un Mapa de objeto e inicializar el deseado a pedido. Por lo tanto, no los inicializa todos al principio, en lugar de llamarlos cuando sea necesario (llamado).

Answer 8

NOTA : La solución de comprobación de doble bloqueo de JohnKlehm es mejor. Dejando esta respuesta aquí por razones históricas.

En realidad debería ser

public synchronized static Object getInstance() {
if (instance == null) {
instance = new Object();
}
return instance;
}