2009-10-28 10 views
10

Mam następującą klasę, która zawiera tylko jedno pole i. Dostęp do tego pola jest chroniony przez blokadę obiektu ("to"). Podczas implementacji equals() muszę zablokować tę instancję (a) i drugą (b). Jeśli wątek 1 wywoła a.equals (b) i jednocześnie wątek 2 wywoła b.equals (a), kolejność blokowania będzie odwrotna w obu implementacjach i może spowodować zakleszczenie.Prawidłowa synchronizacja równań() w języku Java

Jak należy zaimplementować equals() dla klasy, która ma zsynchronizowane pola?

public class Sync { 
    // @GuardedBy("this") 
    private int i = 0; 
    public synchronized int getI() {return i;} 
    public synchronized void setI(int i) {this.i = i;} 

    public int hashCode() { 
     final int prime = 31; 
     int result = 1; 
     synchronized (this) { 
      result = prime * result + i; 
     } 
     return result; 
    } 

    public boolean equals(Object obj) { 
     if (this == obj) 
      return true; 
     if (obj == null) 
      return false; 
     if (getClass() != obj.getClass()) 
      return false; 
     Sync other = (Sync) obj; 
     synchronized (this) { 
      synchronized (other) { 
       // May deadlock if "other" calls 
       // equals() on "this" at the same 
       // time 
       if (i != other.i) 
        return false; 
      } 
     } 
     return true; 
    } 
} 

Odpowiedz

0

Prawidłowa realizacja equals() i hashCode() jest wymagane przez różne rzeczy jak mieszania struktur danych, a więc trzeba tam żadnej realnej opcji. Z innej perspektywy, są to tylko metody z tymi samymi wymaganiami dotyczącymi synchronizacji, co inne metody. Nadal masz problem z zakleszczeniem, ale nie jest to związane z faktem, że to powoduje.

7

Dlaczego warto synchronizować? Jaki jest przypadek użycia, gdy ma znaczenie, jeśli zmienia się on podczas porównania i nie ma znaczenia, czy zmiany następują bezpośrednio po kodzie w zależności od równości. (jeśli masz kod w zależności od właściwości, co stanie się, jeśli wartości staną się nierówne przed lub w trakcie tego kodu)

Myślę, że trzeba spojrzeć na większy proces, aby zobaczyć, gdzie należy zablokować.

+0

Masz rację, że jedna wartość może się zmienić natychmiast po stosunku, ale tak jak wtedy, gdy zawijana wartość jest bardziej interesująca niż prymitywna "int", istnieje możliwość, że ta wartość będzie w niespójnym stanie, chyba że zostanie zsynchronizowana, podczas gdy jej części są badane. –

+1

Prawda, ale nadal masz problem z tym, co zrobić po równym – Mark

+2

w rzeczywistości, kwestia jest widoczność. bez użycia synchronizacji, poprawna kolejność może nadal skutkować nieoczekiwanym wynikiem (tj. wcześniej ustawiona wartość nie jest widoczna, kiedy powinna być). – jtahlborn

1

Zawsze zablokować je w tej samej kolejności, jeden sposób można ustalić kolejność jest na wynikach System.identityHashCode(Object)

Edytuj zawierać komentarz:

najlepszym rozwiązaniem czynienia z rzadkim przypadku IdentyfikatorHashCodes jest równy wymaga więcej szczegółów o tym, jakie inne blokowanie tych obiektów się dzieje.

Wszystkie wymagania dotyczące wielu obiektów wymagają zastosowania tego samego procesu rozdzielczości.

Można utworzyć udostępnione narzędzie do śledzenia obiektów o tym samym kodzie identyfikacyjnym tożsamości dla krótkiego okresu wymagań dotyczących blokady i zapewnić powtarzalne zamówienie dla okresu, w którym są śledzone.

+0

public boolean equals (Object obj) { if (this == obj) return true; if (obj == null) return false; if (getClass()! = Obj.getClass()) return false; Sync other = (Sync) obj; boolean smallerHash = System.identityHashCode (this) Philipp

+0

'System.identityHashCode' nie nadaje unikalnych wartości !! –

9

Próba synchronizacji equals i hashCode wewnątrz obiektu nie będzie działać poprawnie. Rozważmy przypadek HashMap, który używa hashCode, aby odkryć, w którym "wiadrze" znajduje się obiekt, a następnie używa equals do sekwencyjnego przeszukiwania wszystkich obiektów w wiadrze.

Jeśli obiekty mogą mutować w sposób, który zmienia wynikach hashCode lub equals może skończyć się z sytuacji, w której HashMap rozmowy hashCode. Nabiera zamek, dostaje hasz i ponownie zwalnia zamek. HashMap następnie przechodzi do obliczenia, które "wiadro" do użycia. Ale zanim HashMap może uzyskać blokadę równą, ktoś inny chwyta zamek i mutuje obiekt, dzięki czemu equals staje się niespójny z poprzednią wartością hashCode. To doprowadzi do katastrofalnych rezultatów.

Metody hashCode i equals są używane w wielu miejscach i stanowią rdzeń interfejsu API kolekcji Java. Być może warto przemyśleć twoją strukturę aplikacji, która nie wymaga zsynchronizowanego dostępu do tych metod.Lub przynajmniej nie synchronizować się na samym obiekcie.

+0

Dlaczego upadek ?? Jeśli spojrzysz na klasy w JDK (java.util.Date lub java.lang.Long) okaże się, że nie są one również synchronizowane. – leonm

+1

"Jeśli obiekty mogą mutować w sposób, który zmienia wyniki hashCode lub jest równy ..." Większość obiektów robi - nawet java.util.Date (setTime (long)). Zawsze należy o tym pamiętać, używając zaszyfrowanej struktury danych - nie zmieniaj jej po haszymowaniu. Mimo to wolno to zmienić. – sfussenegger

6

Gdzie jest punkt synchronizacji equals(), jeśli wynik nie jest gwarantowane, aby było prawdziwe po synchronizacji pozostało:

if (o1.equals(o2)) { 
    // is o1 still equal to o2? 
} 

Stąd można łatwo zsynchronizować połączenia do getI() wewnątrz równa się jeden po drugim bez zmiany wyjścia - to już nie jest ważne.

Będziesz zawsze trzeba zsynchronizować cały blok:

synchronized(o1) { 
    synchronized(o2) { 
    if (o1.equals(o2)) { 
     // is o1 still equal to o2? 
    } 
    } 
} 

Wprawdzie nadal będziesz twarz ten sam problem, ale przynajmniej Twój synchronizujący w odpowiednim momencie;)

1

Jedynym sposób, aby wiedzieć na pewno, czy synchronizacja jest absolutnie niezbędna, to analiza całego programu w sytuacjach. Są dwie rzeczy, których powinieneś szukać; sytuacje, w których jeden wątek zmienia obiekt, gdy inny wywołuje wartość, oraz sytuacje, w których wątek wywołujący może wywołać nieaktualną wartość i.

Jeśli jednocześnie zablokujesz obiekt this i other, rzeczywiście ryzykujesz zakleszczenie. Ale wątpiłbym, że musisz to zrobić. Zamiast tego, myślę, że należy wdrożyć equals(Object) takiego:

public boolean equals(Object obj) { 
    if (this == obj) 
     return true; 
    if (obj == null) 
     return false; 
    if (getClass() != obj.getClass()) 
     return false; 
    Sync other = (Sync) obj; 
    return this.getI() == other.getI(); 
} 

to nie gwarantuje, że oba obiekty mają taką samą wartość i w tym samym czasie, ale to jest mało prawdopodobne, by żadnego praktycznego znaczenia. W końcu, nawet gdybyś miał tę gwarancję, nadal musiałbyś poradzić sobie z problemem, że te dwa obiekty nie będą już równe w momencie, gdy zostanie zwrócone wywołanie equals. (To jest punkt @!)

Co więcej, nie eliminuje to całkowicie ryzyka zakleszczenia. Rozważmy przypadek, w którym wątek może wywołać equals, przytrzymując zamek jednego z dwóch obiektów; na przykład

// In same class as above ... 
public synchronized void frobbitt(Object other) { 
    if (this.equals(other)) { 
     ... 
    } 
} 

Teraz, jeśli dwa wątki zadzwonić a.frobbitt(b) i b.frobbitt(a) odpowiednio, istnieje ryzyko zakleszczenia.

(Jednak trzeba zrobić, aby wywołać getI() lub zadeklarować i być volatile, inaczej equals() widział nieświeży wartość i jeśli został niedawno zaktualizowany przez inny wątek).

To powiedziawszy, jest coś raczej martwiącego o opartej na wartości metodzie equals na obiekcie, którego wartości składowe mogą być zmutowane. Na przykład spowoduje to przerwanie wielu typów kolekcji. Połącz to z wielowątkowością i będziesz miał duże trudności z ustaleniem, czy twój kod jest naprawdę poprawny. Nie mogę przestać myśleć, że lepiej byłoby zmienić metody, aby nie zależały od stanu, który może mutować po tym, jak metody zostały wywołane za pierwszym razem.

+0

> Co więcej, nie eliminuje to całkowicie ryzyka zakleszczenia. Rozważ przypadek, w którym wątek może wywoływać równe wartości, przytrzymując zamek jednego z dwóch obiektów. Nie jestem pewien, w jaki sposób to jest problem. Jeśli jest to blokada tego, żadna prob jako getI() nie będzie miała charakteru wewnętrznego i nie wpłynie na other.getI(). To samo, jeśli jest to zamek drugiego. Tylko dla odniesienia, jak inni to robią: Wektor synchronizuje się na tym, ale nie na drugim w równości(). Może to prowadzić do ConcurrentModificationException, jeśli wektor zostanie zmodyfikowany podczas uruchamiania funkcji equals(). AtomicInteger nie implementuje equals() – Philipp

+0

@Philipp - potrzebujesz dwóch wątków robiących to z tą samą parą obiektów w przeciwnych pozycjach, aby uzyskać zakleszczenie. –

+0

Czy this.getI() nie zwolni blokady przed wywołaniem metody other.getI()? Nie rozumiem, jak to może się zdarzyć. – Philipp

1

(zakładam, że jesteś zainteresowany w ogólnym przypadku tutaj, a nie tylko w owiniętych liczb całkowitych).

Nie można zapobiec dwa wątki z wywołaniem set ... metody w dowolnej kolejności. Więc nawet gdy jeden wątek dostaje (ważny) true z połączenia .equals( ...), wynik ten może zostać natychmiast unieważniony przez inny wątek, który wywołuje set ... na jednym z obiektów. IOW wynik oznacza tylko, że wartości były równe w momencie porównania.

Dlatego synchronizacji będzie chronić przed przypadku zawiniętego wartości będącej w stanie niespójnym podczas próby zrobić to porównanie (na przykład dwa int -sized połówki zawiniętej long aktualizowany kolejno). Można uniknąć warunku wyścigu, kopiując każdą wartość (tj. Niezależnie synchronizując, bez nakładania się), a następnie porównując kopie.

-2

Odczytuje i zapisuje do int zmienne są już atomowe, więc nie ma potrzeby synchronizowania gettera i setera (patrz http://java.sun.com/docs/books/tutorial/essential/concurrency/atomic.html).

Podobnie, nie trzeba tutaj synchronizować equals. Chociaż można zapobiec zmianie jednego z wartości jednego z wartości i podczas porównania, wątek ten po prostu zostanie zablokowany do momentu, w którym metoda equals zakończy się i od razu zmieni.

0

Jak wskazuje Jason Day, porównywane liczby całkowite są już atomowe, więc synchronizacja jest tutaj zbędna. Ale jeśli tworzyłeś uproszczony przykład, w rzeczywistości myślisz o bardziej złożonym obiekcie:

Bezpośrednia odpowiedź na twoje pytanie jest, upewnij się, że zawsze porównujesz elementy w spójnej kolejności. Nie ma znaczenia, jaka jest ta kolejność, o ile jest spójna. W takim wypadku, System.identifyHashCode stanowiłyby zamawianie, jak:

public boolean equals(Object o) 
{ 
    if (this==o) 
    return true; 
    if (o==null || !o instanceof Sync) 
    return false; 
    Sync so=(Sync) o; 
    if (System.identityHashCode(this)<System.identityHashCode(o)) 
    { 
    synchronized (this) 
    { 
     synchronized (o) 
     { 
     return equalsHelper(o); 
     } 
    } 
    } 
    else 
    { 
    synchronized (o) 
    { 
     synchronized (this) 
     { 
     return equalsHelper(o); 
     } 
    } 
    } 
} 

Następnie zadeklarować equalsHelper prywatne i niech to zrobić prawdziwą pracę porównywania.

(Ale wow, to dużo kodu dla takiego błahego problemu.)

Zauważ, że to zadziałało, każda funkcja, która może zmienić stan obiektu musiałaby zostać uznana zsynchronizowane.

Inną opcją jest synchronizacja w klasie Sync.class, a nie w obu obiektach, a następnie synchronizacja wszystkich ustawiaczy na Sync.class. To zablokuje wszystko na jednym muteksie i uniknie całego problemu. Oczywiście, w zależności od tego, co robisz, może to spowodować niepożądane blokowanie niektórych wątków. Musiałbyś przemyśleć konsekwencje w świetle tego, o czym jest twój program.

Jeśli jest to poważny problem w projekcie, nad którym pracujesz, poważną alternatywą do rozważenia byłoby uniezależnienie obiektu. Pomyśl o String i StringBuilder. Można utworzyć obiekt SyncBuilder, który pozwala wykonać dowolną pracę, aby utworzyć jedną z tych rzeczy, a następnie mieć obiekt Sync, którego stan jest ustawiony przez konstruktor i nigdy nie może się zmienić. Utwórz konstruktor, który pobiera moduł SyncBuilder i ustawia jego stan, aby był zgodny z metodą SyncBuilder.toSync. Tak czy inaczej, robisz cały budynek w SyncBuilder, a następnie przekształcasz go w synchronizację, a teraz masz zagwarantowaną niezmienność, więc nie musisz w ogóle robić bałaganu z synchronizacją.

3

Jeśli zostało to powiedziane wystarczająco, pola używane dla hashCode(), equals() lub compareTo() powinny być niezmienne, najlepiej ostateczne. W takim przypadku nie trzeba ich synchronizować.

Jedynym powodem zaimplementowania funkcji hashCode() jest to, że obiekt można dodać do kolekcji hash i nie można poprawnie zmienić hashCode() obiektu, który został dodany do takiej kolekcji.

+1

Podobnie jak obliczenia dla ArrayList? ;-) Żarty na bok, niezmienny finał to dobra rzecz, do której należy dążyć (prawdopodobnie poza dobrą rzeczą - wszystko jest * tak * znacznie łatwiejsze, gdy używamy niezmiennych obiektów), ale nie zawsze jest to możliwe przy dużych złożonych obiektach. –

+0

Jak korzystać z hashCode() z ArrayList? O ile nie jest możliwe uniezależnienie obiektów, nie zmienia to faktu, że nie można zmienić hashCode() klucza mapy lub elementów zestawu i sprawić, by działał. –

0

Nie używaj synchronizacji. Pomyśl o niemodyfikowalnych ziarnach.

2

Próbujesz zdefiniować oparte na treści "równe" i "hashCode" na zmiennym obiekcie. Jest to nie tylko niemożliwe: nie ma sensu. Według

http://java.sun.com/javase/6/docs/api/java/lang/Object.html

zarówno „równa” i „hashCode” muszą być zgodne: powrót tą samą wartość dla kolejnych wywołań tego samego obiektu (obiektów). Mutability z definicji to zapobiega. To nie jest tylko teoria: wiele innych klas (np. Kolekcje) zależy od obiektów implementujących poprawną semantykę dla equals/hashCode.

Problem synchronizacji jest tutaj czerwonym śledziem. Gdy rozwiążesz podstawowy problem (zmienność), nie będziesz musiał synchronizować. Jeśli nie rozwiążesz problemu zmienności, nie pomoże ci żadna synchronizacja.

+3

Zmienione obiekty z JVM implementują equals (przykład: ArrayList, HashMap), więc twoje stwierdzenie, że "content-based" jest równe "(...) na zmiennym obiekcie (...) nie ma sensu" nie jest udostępniane przez tych, którzy piszą maszynę JVM. Nawet niektóre zsychronizowane klasy zmienne implementują równe wartości (np. Wektor, który nie jest bezpieczny w skutkach). – Philipp

+1

To dobra uwaga. Jednakże, argumentowałbym, że to osoba wywołująca jest odpowiedzialna za uczynienie tych obiektów efektywnymi niezmiennymi przez cały czas trwania operacji wykorzystującej equals/hashCode. Na przykład wywołanie List .contains (x) podczas mutowania x lub mutacji kluczy w Map Ma niezdefiniowane zachowanie. – user58804

0

Należy się upewnić, że obiekty nie zmieniają się między wywołaniami funkcji hashCode() i equals() (jeśli są wywoływane). Następnie musisz upewnić się, że obiekty nie ulegają zmianie (do rozszerzenia, którego dotyczy hashCode i equals), podczas gdy obiekt znajduje się w mapie mieszającej. Aby zmienić obiekt, musisz najpierw go usunąć, a następnie zmienić i odłożyć.

0

Jak wspomnieli inni, jeśli rzeczy ulegną zmianie podczas sprawdzania równości, istnieje już możliwość szalonego zachowania (nawet przy prawidłowej synchronizacji). więc wszystko, co naprawdę musisz się martwić, to widoczność (chcesz się upewnić, że zmiana, która "dzieje się wcześniej", oznacza, że ​​twoje równe połączenie jest widoczne). w związku z tym, można po prostu zrobić „snapshot” równa które będą poprawne pod względem „dzieje się przed” związku i nie będzie cierpieć z powodu problemów blokady Kolejność:

public boolean equals(Object o) { 
    // ... standard boilerplate here ... 

    // take a "snapshot" (acquire and release each lock in turn) 
    int myI = getI(); 
    int otherI = ((Sync)o).getI(); 

    // and compare (no locks held at this point) 
    return myI == otherI; 
}