Java Thread-Sicherheit: Wird hier eine Synchronisierung benötigt? (theoretisch)

[CyborgBeta]

Guten Morgen,

ich würde gerne wissen, ob folgendes Konstrukt eine valide Lösung im Sinne von Javas Concurrency-Konzept darstellt, oder ob es hier theoretisch zu Problemen, sprich data loss, kommen kann:

import java.util.HashMap;
import java.util.Map;
import java.util.Random;

public class STest {
    private static volatile int last_inserted_1;
    private static volatile int last_inserted_2;

    public static void main(String[] args) {
        Map<Integer, Integer> myNotSynchronizedConcurrentMap = new HashMap<>();
        Thread t1 = new Thread(() -> {
            Random rand = new Random();
            for (int i = 0; i < 10_000; i++) {
                last_inserted_1 = rand.nextInt();
                myNotSynchronizedConcurrentMap.put(42, last_inserted_1);
            }
        });
        Thread t2 = new Thread(() -> {
            Random rand = new Random();
            for (int i = 0; i < 10_000; i++) {
                last_inserted_2 = rand.nextInt();
                myNotSynchronizedConcurrentMap.put(43, last_inserted_2);
            }
        });
        t1.start();
        t2.start();
        try {
            t1.join();
            t2.join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println(myNotSynchronizedConcurrentMap);
        System.out.println(last_inserted_1);
        System.out.println(last_inserted_2);
    }
}

Man soll ja, wenn möglich, nur sehr sparsam mit synchronisierten Codeabschnitten umgehen ...

 

[Ray519]

Du nutzt eine normale HashMap. Die ist explizit dokumentiert, nicht für "Concurrent" Zugriffe designed zu sein. Dafür gibt es zB Concurrent SynchronizedMap Wrapper. Oder ganz andere Datenstrukturen die tatsächlich "Concurrent" sind.

Man soll ja, wenn möglich, nur sehr sparsam mit synchronisierten Codeabschnitten umgehen ...

Du hast aber gar keine Synchronisierung. Aber es werden schreibbare Daten zwischen beiden Threads geteilt. Also muss die gewählte Datenstruktur das ausbaden. -> man muss verstehen wie die HashMap funktioniert und was für Garantien die macht.

 

[CyborgBeta]

@Ray519 Danke, aber dein Beitrag ist für mich nicht hilfreich, weil ich jetzt genauso schlau wie vorher bin.

 

[NJay]

@CyborgBeta Dann musst du dich tiefer in Threading einarbeiten, denn es wird in seinem Beitrag sehr kurzum eindeutig auf die Probleme hingewiesen. Das kann an nicht wirklich einfacher erklären ohne dir Threading und dessen Konzepte komplett zu erklären.

 

[CyborgBeta]

Dann musst du dich tiefer in Threading einarbeiten, denn es wird in seinem Beitrag sehr kurzum eindeutig auf die Probleme hingewiesen.

Nein, hat er nicht. Er hat einfach etwas Falsches behauptet und so getan, als ob ich dumm sei und das nicht merken würde:

nicht für "Concurrent" Zugriffe designed zu sein.

dem widerspreche ich.

 

[Ray519]

Jetzt wird es aber krass:

https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html

Noch nicht mal lesen zu können, wenn ich schon die relevanten Hinweise gebe...

 

[moa]

Klingt wie ne Hausaufgabe.
Ich denke, er muss begründen, warum es selbst bei konkurrierendem Zugriff mit unterschiedlichen Keys eine schlechte Idee ist.

 

[CyborgBeta]

Ich hatte nur danach gefragt, ob mein konkreter Anwendungsfall synchronisiert werden muss, und offenbar ist das nicht der Fall:

import java.util.HashMap;
import java.util.Map;
import java.util.Random;

public class STest {
    private static volatile int last_inserted_1;
    private static volatile int last_inserted_2;

    public static void main(String[] args) throws InterruptedException {
        Map<Integer, Integer> myNotSynchronizedConcurrentMap = new HashMap<>();
        Thread t1 = new Thread(() -> {
            Random rand = new Random();
            last_inserted_1 = rand.nextInt();
            myNotSynchronizedConcurrentMap.put(42, last_inserted_1);
            for (int i = 0; i < 10_000; i++) {
                int lastInserted = myNotSynchronizedConcurrentMap.get(42);
                if (lastInserted != last_inserted_1) {
                    throw new RuntimeException();
                }
                last_inserted_1 = rand.nextInt();
                myNotSynchronizedConcurrentMap.put(42, last_inserted_1);
            }
        });
        Thread t2 = new Thread(() -> {
            Random rand = new Random();
            last_inserted_2 = rand.nextInt();
            myNotSynchronizedConcurrentMap.put(43, last_inserted_2);
            for (int i = 0; i < 10_000; i++) {
                int lastInserted = myNotSynchronizedConcurrentMap.get(43);
                if (lastInserted != last_inserted_2) {
                    throw new RuntimeException();
                }
                last_inserted_2 = rand.nextInt();
                myNotSynchronizedConcurrentMap.put(43, last_inserted_2);
            }
        });
        t1.start();
        t2.start();
        t1.join();
        t2.join();
        System.out.println(myNotSynchronizedConcurrentMap);
        System.out.println(last_inserted_1);
        System.out.println(last_inserted_2);
    }
}

Klingt wie ne Hausaufgabe.

Nein, ist keine.

Noch nicht mal lesen zu können, wenn ich schon die relevanten Hinweise gebe

Leider sind die von dir gegebenen Hinweise nicht zielführend bzw. nur am Rand relevant.

Aber gut, ich kann ja nicht lesen. 🤷‍♂️

 

[Tornhoof]

und offenbar ist das nicht der Fall:

Du kannst dein Codebeispiel natürlich noch weiter verändern, aber wieso bist du der Meinung das es nciht der Fall ist?

Wenn du denkst das eine Exception zu schmeißen ein passender Test ist, bist du aufm Holzweg, nur weil es in deinem Code nicht dazu kommt, sagt es nichts über das Design und den Einsatzzweck der Datenstruktur aus.
Zum Bleistift kann im Debug Mode eine andere Struktur genutzt werden, im Release Modus eine andere (inkl. Optimierungen durch den Compiler oder ähnliches), kann auch einfach sein dass zwei Threads zu "langsam" sind.

Es zählt bei Threading und concurrent Zugriff nur eins und das ist die Dokumentation der Struktur.

 

[CyborgBeta]

aber wieso bist du der Meinung das es nciht der Fall ist?

Weil bei 2 Threads mit je 10.000 Iterationen und 12-Core-CPU bislang kein Fehler auftrat ... das ist dann natürlich noch kein Beweis, aber es ist für mich naheliegend.

 

[Tornhoof]

Du machst das noch nicht lange oder?
Aus langer (und teils schmerzvoller) Erfahrung, Threading Fehler zu debuggen ist fast unmöglich, wenn die Datenstruktur sagt, sie ist nicht für den Einsatzzweck geeignet, dann ist sie nicht geeignet. Amen.

 

[CyborgBeta]

Edit: Auch bei 10 Threads mit je 1 Million Iterationen ist dies nicht der Fall:

import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Random;

public class STest {
    private static final int NUMBER_OF_THREADS = 10;
    private static final int[] last_inserted = new int[NUMBER_OF_THREADS];

    public static void main(String[] args) throws InterruptedException {
        Map<Integer, Integer> myNotSynchronizedConcurrentMap = new HashMap<>();
        for (int i = 0; i < NUMBER_OF_THREADS; i++) {
            int finalI = i;
            myNotSynchronizedConcurrentMap.put(i, 0);
            new Thread(() -> {
                Random rand = new Random();
                for (int j = 0; j < 1_000_000; j++) {
                    int lastInserted = myNotSynchronizedConcurrentMap.get(finalI);
                    if (lastInserted != last_inserted[finalI]) {
                        throw new RuntimeException();
                    }
                    last_inserted[finalI] = rand.nextInt();
                    myNotSynchronizedConcurrentMap.put(finalI, last_inserted[finalI]);
                }
            }).start();
        }
        Thread.sleep(10_000);
        System.out.println(Arrays.toString(last_inserted));
        System.out.println(myNotSynchronizedConcurrentMap);
    }
}

wenn die Datenstruktur sagt, sie ist nicht für den Einsatzzweck geeignet, dann ist sie nicht geeignet. Amen.

- [ ] Du weißt, dass es Ausnahmen gibt?

 

[Ray519]

das ist dann natürlich noch kein Beweis, aber es ist für mich naheliegend.

Dann hast du immer noch nicht den Kern meiner Aussage verstanden.
Du denkst du bist schlauer als die Doku der Datenstruktur die du verwendest. Aber du weigerst dich beständig diese Doku zu lesen und zu verstehen wie die Datenstruktur denn überhaupt funktioniert. Wenn man das nämlich weiß, weiß man auch dass du keinen guten Test geschrieben hast um den Fall der kaputt geht wahrscheinlich zu machen.

Stichworte: Hashkollision und Resize.

Auch ist einfach sehr gewagt, zu behaupten die Doku einer JDK Datenstruktur falsch ist, wenn man nicht weiß wie die Datenstruktur funktioniert und das die ConcurrentModificationExceptions die die Datenstruktur wirft "best-effort" sind, sprich die Datenstruktur nicht garantiert dir einen Fehler zu werfen, wenn du sie absichtlich missbrauchst.

Das ist genauso wie die Map zu ändern (mit put zB), während du einen Iterator auf die Map hälst. Das ist nicht ok, die Doku sagt das auch klipp und klar, aber es kann eine ConcurrentModificationException werfen, muss aber nicht.

 

[Sn1ff3r]

Wie in der Doku oben erwähnt kommt es nur bei Strukturänderungen zu Problemen.
Deine Iterationen ändern daran nichts deshalb kannst du die rauf drehen so hoch du willst.
Jeder Thread macht das genau 1x beim ersten put.
Da diese dann hintereinander gestartet werden und jeder Thread seinen eigenen Key hat wird wohl durch Zufall in 99,9% kein Problem auftrreten.

Das ändert noch immer nichts daran dass du sobald dein code komplexer wird (zB hinzufügen desselben keys in 2 parallelen Threads) in Probleme laufen wirst da der Datentyp den du verwendest dafür nicht geeignet ist.

 

[Ray519]

Sprich: wenn du nur genau versuchst die Keys 42 und 43 Concurrent zu ändern und die Map nie ihre Größe ändert, und das jeweils exklusiv von einem Thread passiert, wird bei der Implementierung nie etwas passieren. Aber das ist ja ein Schwachsinniges Beispiel, weil dafür braucht und will man keine HashMap. Und man muss dafür tief verstehen, wie die HashMap intern funktioniert um auch ja keinen Randfall zu übersehen in dem es einem um die Ohren fliegt.


Und PS: die Begründung wieso das gehen würde mit unrealtistischen Access Patterns habe ich sogar auch schon in meinem Post angesprochen.

 

[KeepCalm]

Meine Meinung....die Operationen auf die Map sind über den Key disjunct, deswegen stellt sich hier doch das Threaddingproblem noch nicht

 

[CyborgBeta]

Stichworte: Hashkollision und Resize.

Ersteres wird doch durch die festen Schlüssel ausgeschlossen und Zweiteres lässt sich wahrscheinlich nicht einfach forcieren. Aber jetzt hast du endlich mal Argumente gebracht.

 

[Tornhoof]

dit: Auch bei 10 Threads mit je 1 Million Iterationen ist dies nicht der Fall:

Probier mal mehr Threads, 16, 32, 64,128 oder so

 

[Ray519]

Und weißt du was eine kompetente Antwort auf mein ersten Post gewesen wäre:
"Hast du denn auch berücksichtigt, dass ich nur statische Int-Keys nutze, getrennt pro Thread und ich nicht Plane die HashMap auf irgendeine andere Weise zu nutzen, die für eine HashMap angemessen wäre?". Ich habe tatsächlich nicht gesehen, dass du immer den selben Key nutzt pro Thread. Weil es den Zweck einer HashMap komplett untergräbt.

Aber es bleibt dabei: das Beispiel macht für eine HashMap null Sinn. Deshalb, ohne Versprechen, dass das Beispiel genau nur für den extrem künstlichen Kontext hier sein soll, gebe ich eine Antwort für die HashMap allgemein, wenn du darauf schreibst und die HashMap in mehrere Threads leakst.

 

[CyborgBeta]

Mach mal 64+ Threads

Danke. Jetzt ist bei mir auch gerade der Fehler aufgetreten:

import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Random;

public class STest {
    private static final int NUMBER_OF_THREADS = 1000;
    private static final int[] last_inserted = new int[NUMBER_OF_THREADS];

    public static void main(String[] args) throws InterruptedException {
        Map<Integer, Integer> myNotSynchronizedConcurrentMap = new HashMap<>();
        for (int i = 0; i < NUMBER_OF_THREADS; i++) {
            int finalI = i;
            myNotSynchronizedConcurrentMap.put(finalI, 0);
            new Thread(() -> {
                Random rand = new Random();
                for (int j = 0; j < 10_000; j++) {
                    int lastInserted = myNotSynchronizedConcurrentMap.get(finalI);
                    if (lastInserted != last_inserted[finalI]) {
                        throw new RuntimeException();
                    }
                    last_inserted[finalI] = rand.nextInt();
                    myNotSynchronizedConcurrentMap.put(finalI, last_inserted[finalI]);
                }
            }).start();
        }
        Thread.sleep(100_000);
        System.out.println(Arrays.toString(last_inserted));
        System.out.println(myNotSynchronizedConcurrentMap);
    }
}