Safe publication - czy dużo się zmieniło od legendarnej książki?

Cześć, po przeczytaniu "Java concurrency in Practice" bawię kilkoma rzeczami, które miałem potrzebę samemu sprawdzić eksperymentalnie.

Przede wszystkim najmocniej analizowałem safe publication podczas czytania.
Wtajemniczeni kojarzą być może taki obiekt:

public class Holder {
    private int n;

    public Holder(int n) {
        this.n = n;
    }

    public void assertSanity() {
        if (n != n)
            throw new AssertionError("This statement is false.");
    }
}

Napisałem sobie jakiegoś callera:

public class Main {

    private Holder holder;

    void execute() throws InterruptedException {
        final ExecutorService executorService = Executors.newFixedThreadPool(100);

        final Thread thread = new Thread(() -> holder = new Holder(1000));
//        thread.start(); // A
//        thread.join(); // A+B
        holder = new Holder(1000); // C

        for (int i = 0; i < 1000; ++i) {
            executorService.execute(() -> holder.assertSanity());
        }

        executorService.shutdown();
    }

    public static void main(String[] args) throws InterruptedException {
        new Main().execute();
    }
}

I teraz tak ...
A - mamy unsafe publication, bo przetestowałem i czasami leci NPE na holderze jak executor w innych wątkach odpala na nim assertSanity. Nigdy jednak nie udało mi się dostać wyjątku AssertionError czyli, że referencja do holdera była, ale stan był niespójny. Być może po prostu taka sytuacja na nowych javach ma miejsce wyjątkowo rzadko?

A + B - tutaj nie udało mi się wywalić niczego, zawsze działa. Dlaczego? Przecież wątek callera to inny wątek niż te w executorze i mimo, że czekam aż się ten wątek inicjujący holdera wykona to one zawsze go widzą? To znowu efekt super nowych JDK?

C - tutaj podobnie jak w A+B, nie udało mi się wywalić i pewnie sprawa jest podobna jak w pkt poprzednim?

Pytanie bonus ...
Skoro konstruktory nie są synchronizowane to jeśli tworzymy obiekt, który będzie zaraz czytany przez wiele innych wątków to czy referencje nie powinna być zawsze final lub volatile? Dodatkowo jeśli ten obiekt jest mutowalny to czy w konstruktorze nie trzeba założyć locka? Albo zrobić ewentualnie metodę fabrykującą jako synchronized?

Cząstkowa odp: jak długo trwa wykonanie kodu konstruktora, nikt inny jeszcze nie zna referencji do obiektu, więc w/w kłopoty nie mogą zajść.
Techniki synchronizacyjne w konstruktorze MSZ nie są konieczne. W zwykłych metodach być może tak

Ogólnie:

1. Java Concurrency in Practice jest dosyć wiekowa. To, co w niej jest jest jak najbardziej prawdziwe, natomiast powstały nowsze zabawki rozwiązujące te problemy.
2. Twój test jest bardzo dziwny bo tak naprawdę nie wiadomo, co testuje.

Masz dwa wątki (executora pomijam bo to nie ma nic do rzeczy):

• #1 - wątek główny
• #2 - wątek obsługujący Thread

Teraz: konstruktor Thread(Runnable) nie wykonuje przekazanego Runnable od razu - tylko po utworzeniu wątku. W rezultacie wątek główny ma szansę, że "dobiegnie" do linijki z assertSanity zanim wykona się Runnable przekazane do Thread.

W przypadku A+B masz Thread.join() - czyli wątek #1 poczeka na #2. W rezultacie Runnable zawsze wykona się przed wykonaniem linijki z asercją.
W przypadku C pole holder zostanie zainicjalizowane jeszcze przed asercją.

Jeśli chodzi o pytanie "bonus" - to zależy co chcesz osiągnąć.

1. Jeśli obiekt jest niemutowalny to musisz jedynie zapewnić, że będzie zainicjalizowany przed czytaniem.
2. Jeśli obiekt jest mutowalny, ale nie zależy ci na synchronizacji - to jak wyżej.
3. Jeśli obiekt jest mutowalny i zależy ci na synchronizacji - to możesz użyć synchronize, użyć zmiennej atomowej (AtomicReference, AtomicInteger itp.), użyć locka, użyć volatile i pewnie jeszcze kilka innych opcji by się znalazło.

@wartek01:
Ale odczyt holdera czyli wywołanie metody test assertSanity dzieje się w innych wątkach bez żadnej synchronizacji i z tego co wiem jest to niebezpieczne nawet jak wcześniej już konstruktor się zakończył.

@wartek01: Ale właśnie chodzi o to, że executor odpala to w innych wątkach niż utworzenie Holdera. I NPE leci tylko w przypadku A a na moje w przypadku C oraz A+B też powinien.

Przecież widoczność między wątkami nie jest taka, że zawsze widzi najnowsze wartości.

@Bambo: nie, nie chodzi. Kolejne instrukcje kodu są odpalane w głównym wątku, obecność ExecutorService nie ma tutaj nic do rzeczy.

To, że w przypadku A dostajesz NPE wynika z tego, że główny wątek dobiega do asercji zanim ten drugi wątek się wykona. W przypadku A+B i C to nie występuje bo albo to synchronizujesz, albo inicjalizujesz holder'a w głównym wątku.
Dorzuć Thread.sleep(1000) przed forem i też NPE zniknie.

@wartek01:
Przeanalizowałem to ze znajomym co robi w watkowosci i wg niego wszystkie 3 przypadki są unsafe xd

A - to już napisałeś
B - robisz .join() - join to synchronization point, JLS zapewnia, że wszystko co przed, będzie widoczne jak join() zwróci sterowanie do programu
C - w executorze startujesz wątki, start() to również synchronization point, więc to samo co wyżej

https://docs.oracle.com/javase/specs/jls/se11/html/jls-17.html#jls-17.4.5

It follows from the above definitions that:

An unlock on a monitor happens-before every subsequent lock on that monitor.

A write to a volatile field (§8.3.1.4) happens-before every subsequent read of that field.

A call to start() on a thread happens-before any actions in the started thread.

All actions in a thread happen-before any other thread successfully returns from a join() on that thread.

The default initialization of any object happens-before any other actions (other than default-writes) of a program.

Jeśli dobrze zrozumiałem pytania, to 2/3 przypadki są w tym konkretnym kodzie SAFE.
Co do pytania bonusowego, to nie, nie trzeba dlatego że start() zapewnia widoczność wszystkiego co było przed. Gdybyś wcześniej stworzył ten wątek a później chciał tylko odczytać dane, czyli pominał synchronization point, to byłoby unsafe

@pedegie:
Dzięki! W książce pominąłem, że start() na Thread oraz join() jest pkt synchronizacji. To jest to happens-before tak?

Mówisz, że konstruktor nie wymaga synchronizacji a jak się odniesiesz do tego wątku https://stackoverflow.com/questions/10528572/java-concurrency-in-practice-sample-14-12 ??

Gdybyś wcześniej stworzył ten wątek a później chciał tylko odczytać dane, czyli pominał synchronization point, to byłoby unsafe

Tutaj nie bardzo rozumiem jak to miało by wyglądać. Mógłbyś na kodzie to pokazać jeśli masz czas oczywiście?

Generalnie prosiłbym też o przykład unsafe publication tak żeby to asset sanity się wykrzaczyło.

Bambo napisał(a):

Dzięki! W książce pominąłem, że start() na Thread oraz join() jest pkt synchronizacji. To jest to happens-before tak?

happens-before to określenie relacji kolejności wykonywanego kodu - tutaj masz całkiem dobre dwie prezentacje w tej tematyce:

https://www.youtube.com/watch?v=TK-7GCCDF_I
https://www.youtube.com/watch?v=TxqsKzxyySo

Mówisz, że konstruktor nie wymaga synchronizacji a jak się odniesiesz do tego wątku https://stackoverflow.com/questions/10528572/java-concurrency-in-practice-sample-14-12 ??

Ta klasa (i ten przykład z książki) nie odnosi się do żadnego szerszego kontekstu, autor pokazuję jak stworzyć thread-safe Semaphore. Zauważ, że jak tworzymy jakąś klaskę na użytek ogólny (jakieś API po prostu) to nie mamy pojęcia w jaki sposób użytkownicy będą z naszej biblioteki /klasy korzystać. Gdybyśmy mieli gwarancję, że użytkownik tej klasy zawsze najpierw stworzy jej instancję -> następnie będzie synchronization point -> a dopiero póżniej inne wątki będą sie do tego odwołoywać, to ten lock w konstruktorze nie byłby potrzebny. Innymi słowy nie wiesz czy ktoś użyje tej klasy tak:

semaphore = new Semaphore(5)    // semaphore jest polem klasy, nie lokalnym metody ofc  
Thread t = new Thread(() -> sleep(10); semaphore.cosTam());
t.start();

Czy może jednak tak:

Thread t = new Thread(() -> sleep(10); semaphore.cosTam());
t.start();
semaphore = new Semaphore(5)     // X

W drugim przypadku, nasz synchronization point był jeszcze przed inicjalizacją semaphore - więc nie mamy w stosunku do niego relacji happens-before, patrząc z perspektywy wątku w którym jest instrukcja "X".
I teraz pomimo tego, że spokojnie zdąży się wykonać instrukcja oznaczona "X" zanim t się obudzi - to możemy właśnie dostać jakiegoś null pointer'a lub zastać pole permits z domyślną wartością 0.

Podsumowując: skoro nie wiem, w jaki sposób będzie moja klasa używana - to powinienem ją napisać w taki sposób, żeby działała zawsze. Więc w tym przypadku pole semaphore mogłoby być oznaczone jako volatile, żeby uniknąć NPE a dodatkowo lock w konstruktorze semaphore'a, żeby mieć pewność że tam na pewno będzie 5 a nie 0. W ten sposób obsłużyliśmy 100% przypadków związanych z synchronizacją tego obiektu.

Gdybyś wcześniej stworzył ten wątek a później chciał tylko odczytać dane, czyli pominał synchronization point, to byłoby unsafe

Tutaj nie bardzo rozumiem jak to miało by wyglądać. Mógłbyś na kodzie to pokazać jeśli masz czas oczywiście?

W zasadzie zreprodukowałeś to już dostając NPE. Chodziło mi o to, że najpierw startujemy wątek -> następnie przypisujemy coś tam do referencji holder a dopiero później z tego** już uruchomionego** wątku próbujemy coś odczytac z holder

Generalnie prosiłbym też o przykład unsafe publication tak żeby to asset sanity się wykrzaczyło.

To prawie graniczy z cudem, żeby ten konkretny przykład z holderem się wywalilł (próbowałem :P), dlatego że:

https://stackoverflow.com/a/40182163/10528516

Of course, according to Murphy’s Law, it will never happen when you try to provoke that error anyway, but once in a while at the customer, but never reproducible…