2011-11-30 14 views
6

Tengo una interfaz común para varias implementaciones de singleton. La interfaz define el método de inicialización que puede arrojar una excepción marcada.Fábrica de objetos singleton: ¿este código es seguro para subprocesos?

Necesito una fábrica que devolverá las implementaciones de singleton en caché bajo demanda, y me pregunto si el siguiente enfoque es seguro para subprocesos

Update1: Por favor, no sugieren ningún parcialmente bibliotecas 3ª, ya que esto requerirá para obtener la autorización legal debido a posibles problemas de licencia :-)

Update2: este código es probable que se utilizará en Entorno EJB, por lo que es preferible no generar hilos adicionales o usar cosas como esa.

interface Singleton 
{ 
    void init() throws SingletonException; 
} 

public class SingletonFactory 
{ 
    private static ConcurrentMap<String, AtomicReference<? extends Singleton>> CACHE = 
     new ConcurrentHashMap<String, AtomicReference<? extends Singleton>>(); 

    public static <T extends Singleton> T getSingletonInstance(Class<T> clazz) 
     throws SingletonException 
    { 
     String key = clazz.getName(); 
     if (CACHE.containsKey(key)) 
     { 
      return readEventually(key); 
     } 

     AtomicReference<T> ref = new AtomicReference<T>(null); 
     if (CACHE.putIfAbsent(key, ref) == null) 
     { 
      try 
      { 
       T instance = clazz.newInstance(); 
       instance.init(); 
       ref.set(instance); // ----- (1) ----- 
       return instance; 
      } 
      catch (Exception e) 
      { 
       throw new SingletonException(e); 
      } 
     } 

     return readEventually(key); 
    } 

    @SuppressWarnings("unchecked") 
    private static <T extends Singleton> T readEventually(String key) 
    { 
     T instance = null; 
     AtomicReference<T> ref = (AtomicReference<T>) CACHE.get(key); 
     do 
     { 
      instance = ref.get(); // ----- (2) ----- 
     } 
     while (instance == null); 
     return instance; 
    } 
} 

No estoy del todo seguro acerca de las líneas (1) y (2). Sé que el objeto referenciado se declara como campo volátil en AtomicReference y, por lo tanto, los cambios realizados en la línea (1) deberían ser inmediatamente visibles en la línea (2), pero todavía tengo algunas dudas ...

Aparte de eso, creo El uso de ConcurrentHashMap aborda la atomicidad de poner una nueva clave en un caché.

¿Vieron alguna preocupación con este enfoque? ¡Gracias!

PS: que sé de lenguaje de clase estática titular - y yo no lo uso debido a ExceptionInInitializerError (que cualquier excepción lanzada durante la instanciación Singleton se envuelve en) y la posterior NoClassDefFoundError que no son algo que quiero coger . En cambio, me gustaría aprovechar la ventaja de la excepción comprobada dedicada al capturarlo y manejarlo con elegancia en lugar de analizar el rastro de la pila de EIIR o NCDFE.

Respuesta

0

Considere el uso de CacheBuilder de Guava. Por ejemplo:

private static Cache<Class<? extends Singleton>, Singleton> singletons = CacheBuilder.newBuilder() 
    .build(
     new CacheLoader<Class<? extends Singleton>, Singleton>() { 
     public Singleton load(Class<? extends Singleton> key) throws SingletonException { 
      try { 
      Singleton singleton = key.newInstance(); 
      singleton.init(); 
      return singleton; 
      } 
      catch (SingletonException se) { 
      throw se; 
      } 
      catch (Exception e) { 
      throw new SingletonException(e); 
      } 
     } 
     }); 

public static <T extends Singleton> T getSingletonInstance(Class<T> clazz) { 
    return (T)singletons.get(clazz); 
} 

Nota: este ejemplo no se ha probado ni compilado.

La implementación subyacente de Cache de Guava manejará toda la lógica de caché y simultaneidad.

+0

Gracias! La lib de terceros no es una opción en mi caso ... – anenvyguest

-1

El código no es generalmente seguro para subprocesos porque hay un espacio entre el CACHE.containsKey(key) y la llamada CACHE.putIfAbsent(key, ref). Es posible que dos hilos llamen simultáneamente al método (especialmente en sistemas de núcleo múltiple/procesador) y ambos realizan la comprobación containsKey(), luego ambos intentan realizar las operaciones de puesta y creación.

Protegería la ejecución del método getSingletonInstnace() utilizando un bloqueo o sincronizando en un monitor de algún tipo.

+1

Protege contra esto al poner 'AtomicReference' con un valor' null'. Si el 'putIfAbsent()' no devuelve null, simplemente lo descarta y las llamadas obtienen. Ese es el punto del ciclo. – Gray

3

Tener todas estas cosas concurrentes/atómicas podría causar problemas de bloqueo más que simplemente poner

synchronized(clazz){} 

bloques alrededor del captador. Las referencias atómicas son para referencias que están ACTUALIZADAS y no desea una colisión. Aquí tienes un único escritor, por lo que no te importa eso.

Se puede optimizar aún más por tener un HashMap, y sólo si hay un fallo, utilice el bloque sincronizado:

public static <T> T get(Class<T> cls){ 
    // No lock try 
    T ref = cache.get(cls); 
    if(ref != null){ 
     return ref; 
    } 
    // Miss, so use create lock 
    synchronized(cls){ // singletons are double created 
     synchronized(cache){ // Prevent table rebuild/transfer contentions -- RARE 
      // Double check create if lock backed up 
      ref = cache.get(cls); 
      if(ref == null){ 
       ref = cls.newInstance(); 
       cache.put(cls,ref); 
      } 
      return ref; 
     } 
    } 
} 
+0

¡Gracias! Estaba considerando este enfoque, pero había un razonamiento en el libro "Concurrencia de Java en la práctica", que los algoritmos atómicos bajo carga moderada superan a aquellos basados ​​en clases de bloqueo que a su vez superan a los basados ​​en bloqueo intrínseco. Sin embargo, me inclino por el código que sugieres debido a que es mucho más legible :-) – anenvyguest

+0

Sí, pero como singleton, nunca presionarás LOAD, porque la creación es la única parte bloqueada. Los captadores no están sincronizados. Y en mi experiencia, girar siempre ha sido peor que bloquear bloqueos. – Nthalk

+0

Además de mi comentario anterior, creo que java.util.HashMap simple no será suficiente aquí ya que 'cache.put (cls, ref)' puede desencadenar la reconstrucción de la tabla interna y, por lo tanto, 'cache.get (cls)' puede vea el mapa en estado inconnsistente, ya que la última llamada se realiza fuera del bloque 'sincronizado'. – anenvyguest

0

Esto parece que podría funcionar aunque podría considerar una especie de sueño si aún un nanosegundo o algo así cuando se prueba que se establezca la referencia. El ciclo de prueba de giro será extremadamente costoso.

Además, consideraría mejorar el código pasando el AtomicReference-readEventually() lo que puede evitar la condición containsKey() y luego putIfAbsent() carrera. Por lo que el código sería:

AtomicReference<T> ref = (AtomicReference<T>) CACHE.get(key); 
if (ref != null) { 
    return readEventually(ref); 
} 

AtomicReference<T> newRef = new AtomicReference<T>(null); 
AtomicReference<T> oldRef = CACHE.putIfAbsent(key, newRef); 
if (oldRef != null) { 
    return readEventually(oldRef); 
} 
... 
3

Has ido a un montón de trabajo para evitar la sincronización, y supongo que la razón para hacer esto es para los problemas de rendimiento. ¿Has probado para ver si esto realmente mejora el rendimiento frente a una solución sincronizada?

La razón por la que pregunto es que las clases concurrentes tienden a ser más lentas que las no concurrentes, por no mencionar el nivel adicional de redirección con la referencia atómica. Dependiendo de la contención de su hilo, una solución sincronizada ingenua puede ser más rápida (y más fácil de verificar si es correcta).

Además, creo que posiblemente puede terminar con un bucle infinito cuando se lanza una SingletonException durante una llamada a instance.init(). La razón es que un subproceso concurrente esperando en readEventually nunca terminará encontrando su instancia (ya que se lanzó una excepción mientras otro subproceso estaba inicializando la instancia). Tal vez este es el comportamiento correcto para su caso, o tal vez desea establecer un valor especial para la instancia para desencadenar una excepción que se lanzará a la cadena de espera.

+0

¡Buena captura!De hecho, me perdí el bucle infinito cuando se lanza una excepción. – anenvyguest

-1

google "Memoizer". básicamente, en lugar de AtomicReference, use Future.

Cuestiones relacionadas