Uno de los errores más comunes que se ven en los proyectos donde he estado es el infame lazy init de variables, especialmente collections. O sea:
private List lista = null;
public List getLista() {
if (lista == null) {
lista = new ArrayList();
}
return lista;
}
No me diréis que no lo habéis visto nunca. La excusa de esto es la «carga perezosa»: no se carga la lista hasta que no se necesita con lo que se ahorra tiempo de cálculo al inicio, memoria y bla, bla, bla.
Esto en un proceso monohilo no tendría mayor problema. El problema es cuando este código está en una clase que ha de ser accesible por muchos hilos simultáneamente, y esto incluye cuando la gestionamos en un bean de Spring o JEE o singletons similares. Puesto que la variable es compartida por las instancias de la clase, todos los hilos acceden a ella sin preocuparse unos de otros. En otras palabras: la clase no es thread safe con lo que hay que sincronizar o tendremos lío asegurado.
El detalle
Mucha gente no ve el porqué. Vamos a verlo con algo más de detalle:
Lo que todo el mundo espera que pase es: el hilo A entra en el método, ve que es null y crea el objeto; el hilo B entra, ve que no es null y lo devuelve ya creado. Si siempre pasase esto no habría ningún problema.
Lo que puede pasar es: el hilo A entra en el método, ve que es null; el hilo B entra simultáneamente y también ve que es null; el hilo A crea el objeto y el hilo B lo vuelve a crear. Aquí tenemos un problema, hemos creado dos objetos diferentes donde solo esperábamos uno.
Solución 1 (Correcta pero poco óptima)
La primera solución, y la más fácil, es sincronizar el método completo:
private List lista = null;
public synchronized List getLista() {
if (lista == null) {
lista = new ArrayList();
}
return lista;
}
Lo cual, a pesar de ser correcto, es muuuuy poco óptimo, ya que lo que obligamos es a poner a todos los hilos en fila y a entrar al método uno a uno siempre, incluso cuando ya hemos creado la lista y no necesitamos sincronizar nada.
Solución 2 (Incorrecta)
Podemos mejorarlo un poco más:
private List lista = null;
public List getLista() {
if (lista == null) {
synchronized(this) {
lista = new ArrayList();
}
}
return lista;
}
Ahora parece mejor, pero ya no es correcto. Por ejemplo, el hilo A y el B entran simultáneamente, como hay un synchronized se ponen en fila (ya no pueden entrar a la vez en el bloque); entra el hilo A y crea la lista, al salir el hilo A del bloque sincronizado el hilo B ya puede entrar y vuelve a crear la lista. Hemos creado dos objetos, el mismo problema de antes.
Solución 3 (Incorrecta)
Y aquí llegamos a la solución que casi todo el mundo considera correcta, la doble comprobación:
private List lista = null;
public List getLista() {
if (lista == null) {
synchronized(this) {
if (lista == null) {
lista = new ArrayList();
}
}
}
return lista;
}
Ahora parece todo correcto, el hilo A crea el objeto, el hilo B entra en el bloque sincronizado, comprueba que el objeto ya ha sido creado y devuelve el objeto creado por el primer hilo. ¡Prueba superada!
Pues no, esto está mal y está mal por algo que el 99.9% de programadores que conozco no se han molestado en aprender, el modificador volatile.
¿Cuál es el problema? El hilo A entra en el bloque sincronizado y empieza a crear la lista, nótese el empieza porque aquí está el quid: puede darse el caso de que lista == null sea false, pero lista todavía no esté correctamente instanciada. Sí, hijos míos, esto es perfectamente posible y así está documentado en las especificaciones de la JVM. Por ejemplo, tenemos una clase con 5 variables int que inicializamos en un constructor. Primero se crea la instancia con las variables en su valor por defecto (0) y luego se les adjudican los valores del constructor. Es posible que un hilo pille la construcción a medias y unas variables estén a 0 y otras no. Ojo, esto solo pasa si las variables inicializadas en el constructor no son final. Un constructor que inicializa variable final es thread safe.
O sea, la lista está compartida por todos los hilos y su lectura no está sincronizada como en la Solución 1 (fijaos que el primer bloque if está fuera del synchronized) por lo que se puede dar el caso de que el hilo A empieza a crear la lista, el hilo B vea lista==null como false y nos la devuelva antes que el hilo A termine de crearla y error al canto.
Solución 4 (Correcta)
Solución, hacer lista volatile. Con volatile nos aseguramos, entre otras cosas, que una instancia que lee un hilo cualquiera esté completamente inicializada y no parcialmente. Así pues, el código correcto sería:
private volatile List lista = null;
public List getLista() {
if (lista == null) {
synchronized(this) {
if (lista == null) {
lista = new ArrayList();
}
}
}
return lista;
}
Solución 5 (Correcta)
Existe una solución todavía mejor ¿Realmente necesitamos una inicialización perezosa? Si la respuesta es no (valga decir que en el 99% de los casos en los que he visto este código la respuesta es no) podemos escribir:
private final List lista = new ArrayList();
public List getLista() {
return lista;
}
Con final también nos aseguramos que no suceda la instanciación parcial de la que hablaba antes y además es más óptimo que volatile. Pensad que cualquier tipo de sincronización supone un coste en proceso. El de volatile es realmente muy poco, pero si se puede evitar, bienvenido sea.
Ojo, olvidarse el final en la lista hace que esto vuelva a no ser thread safe. A pesar de que parezca anti-intuitivo se puede volver a dar el caso anterior, lista está parcialmente instanciada cuando algún hilo la lee, llamando al método getLista, y error otra vez.
Conclusiones
Os oigo los pensamientos: «Pues yo he hecho esto muchas veces y siempre ha ido bien». El hecho de que hasta ahora no te haya fallado no quiere decir que esté bien, quiere decir que nunca te ha fallado.
También escucho los pensamientos del guru: «Pero esto solo pasa en las JVM tal y pascual y yo no las utilizo». Tu código no es portable y lo peor, no puedes estar seguro de que en alguna revisión esto no cambie y que sí que te afecte posteriormente.
En resumen, evitad las cargas perezosas a menos que de verdad se necesiten, utilizad el modificador final siempre que podáis y leeros la documentación porque esto es mucho más complicado de lo que parece. Por ejemplo:
es thread safe, pero
no lo es. (!?)
En la documentación encontraréis el porqué y en volatile la respuesta. Amén.