Fabryka obiektów singleton: czy ten kod jest bezpieczny dla wątków?

Question

Mam wspólny interfejs dla wielu wdrożeń singleton. Interfejs definiuje metodę inicjalizacji, która może rzucić zaznaczony wyjątek.

Potrzebuję fabryki, która na żądanie zwróci buforowane wdrożenia singleton i zastanawiam się, czy następujące podejście jest bezpieczne dla wątków?

UPDATE1: Proszę nie sugerować żadnych 3rd częściowo bibliotek, jak będzie to wymagało, aby uzyskać luz prawnej z powodu możliwych problemów licencyjnych :-)

Update2: kod ten może być stosowany w Środowisko EJB, więc lepiej jest nie spawnować dodatkowych wątków lub używać takich rzeczy.

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; 
    } 
} 

Nie jestem całkowicie pewien linii (1) i (2). Wiem, że obiekt odniesienia jest zadeklarowany jako zmienne pole w AtomicReference, a zatem zmiany wprowadzone w linii (1) powinny być natychmiast widoczne w linii (2) - ale nadal mają pewne wątpliwości ...

Poza tym - myślę użycie adresu ConcurrentHashMap dotyczy atomowości umieszczania nowego klucza w pamięci podręcznej.

Czy widzicie jakieś obawy związane z tym podejściem? Dzięki!

PS: wiem o statycznej klasy uchwyt idiomu - i nie używać go z powodu ExceptionInInitializerError (co każdy wyjątek rzucony podczas singleton instancji jest owinięty w późniejszym NoClassDefFoundError) i które nie są czymś, chcę złapać . Zamiast tego chciałbym wykorzystać zalety dedykowanego sprawdzanego wyjątku, łapiąc go i traktując go z gracją, zamiast analizować ślad stosu EIIR lub NCDFE.

Answer 1

Rozważ użycie Guva CacheBuilder. Na przykład:

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); 
} 

Uwaga: Ten przykład jest niesprawdzone i Nieskompilowany.

Podstawowa implementacja GUavy Cache obsłuży wszystkie logiki buforowania i współbieżności.

Answer 2

Kod nie jest na ogół bezpieczny dla wątków, ponieważ istnieje luka pomiędzy sprawdzaniem CACHE.containsKey(key) i rozmową CACHE.putIfAbsent(key, ref). Możliwe jest jednoczesne wywołanie dwóch wątków w metodzie (szczególnie w systemach wielordzeniowych/procesorach) i oba sprawdzają, sprawdzając, czy oba próbują wykonać operacje wstawiania i tworzenia.

Chciałbym chronić to wykonanie metody getSingletonInstnace() za pomocą blokady lub synchronizacji na jakimś monitorze.

Answer 3

Mając wszystkie te rzeczy jednocześnie/atomowych spowoduje problemów więcej niż tylko wprowadzenie blokady

synchronized(clazz){} 

bloki wokół getter. Atomowe odniesienia są dla odniesień, które są ZAKTUALIZOWANE i nie chcesz kolizji. Tutaj masz jednego pisarza, więc nie przejmujesz się tym.

Można go zoptymalizować dalej poprzez HashMap, i tylko wtedy, gdy nie jest miss, użyj zsynchronizowane bloku:

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; 
     } 
    } 
} 

Answer 4

to wygląda, że ​​to działa, chociaż mógłbym rozważyć jakiś sen, czy nawet nanosekunda lub coś w trakcie testowania referencji do ustawienia. Pętla testu wirowania będzie niezwykle kosztowna.

Również uważam poprawę kodu przez przepuszczenie AtomicReference do readEventually() dzięki czemu można uniknąć stanu containsKey() a następnie putIfAbsent() wyścigu. Więc kod byłoby:

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); 
} 
... 

Answer 5

Ty już do dużo pracy, aby uniknąć synchronizację, i zakładam powód robi to dla problemów z wydajnością. Czy sprawdziłeś, czy to rzeczywiście poprawia wydajność w porównaniu do zsynchronizowanego rozwiązania?

Pytam że jednoczesnego ćwiczenia wydają się być wolniejszy niż non-jednoczesnych te, nie wspominając o dodatkowy poziom przekierowania z odniesieniem atomowej. W zależności od niezgodności wątku naiwne zsynchronizowane rozwiązanie może być w rzeczywistości szybsze (i łatwiejsze do sprawdzenia pod kątem poprawności).

Dodatkowo, myślę, że można ewentualnie skończyć z nieskończonej pętli, gdy SingletonException jest wyrzucany podczas rozmowy do instance.init(). Powodem jest to, że współbieżny wątek oczekujący w readEventually nigdy nie zakończy się znalezieniem jego instancji (ponieważ wyjątek został zgłoszony, gdy inny wątek inicjował instancję). Być może jest to poprawne zachowanie dla twojej sprawy, a może chcesz ustawić specjalną wartość dla instancji, aby wyzwolić wyjątek do rzucenia w oczekujący wątek.

Answer 6

google "Memoizer". zasadniczo zamiast AtomicReference, użyj Future.