Warum wird dieses Java-Programm trotzdem beendet, sollte es anscheinend nicht (und nicht)?

205

Eine sensible Operation in meinem Labor ist heute völlig schief gelaufen. Ein Aktuator auf einem Elektronenmikroskop überschritt seine Grenzen, und nach einer Kette von Ereignissen verlor ich 12 Millionen Dollar an Ausrüstung. Ich habe über 40K Zeilen im fehlerhaften Modul auf diese eingegrenzt:

import java.util.*;

class A {
    static Point currentPos = new Point(1,2);
    static class Point {
        int x;
        int y;
        Point(int x, int y) {
            this.x = x;
            this.y = y;
        }
    }
    public static void main(String[] args) {
        new Thread() {
            void f(Point p) {
                synchronized(this) {}
                if (p.x+1 != p.y) {
                    System.out.println(p.x+" "+p.y);
                    System.exit(1);
                }
            }
            @Override
            public void run() {
                while (currentPos == null);
                while (true)
                    f(currentPos);
            }
        }.start();
        while (true)
            currentPos = new Point(currentPos.x+1, currentPos.y+1);
    }
}

Einige Beispiele der Ausgabe, die ich bekomme:

$ java A
145281 145282
$ java A
141373 141374
$ java A
49251 49252
$ java A
47007 47008
$ java A
47427 47428
$ java A
154800 154801
$ java A
34822 34823
$ java A
127271 127272
$ java A
63650 63651

Da es hier keine Gleitkomma-Arithmetik gibt und wir alle wissen, dass sich vorzeichenbehaftete Ganzzahlen beim Überlauf in Java gut verhalten, würde ich denken, dass an diesem Code nichts falsch ist. Obwohl die Ausgabe anzeigt, dass das Programm die Beendigungsbedingung nicht erreicht hat, hat es die Beendigungsbedingung erreicht (es wurde sowohl erreicht als auch nicht erreicht?). Warum?


Ich habe festgestellt, dass dies in einigen Umgebungen nicht der Fall ist. Ich bin auf OpenJDK 6 unter 64-Bit-Linux.

Hund
quelle
41
12 Millionen Geräte? Ich bin wirklich neugierig, wie das passieren könnte ... warum Sie einen leeren Synchronisationsblock verwenden: synchronized (this) {}?
Martin V.
84
Dies ist nicht einmal remote threadsicher.
Matt Ball
8
Interessant zu beachten: Hinzufügen des finalQualifikators (der keinen Einfluss auf den erzeugten Bytecode hat) zu den Feldern xund y"Beheben" des Fehlers. Obwohl dies keinen Einfluss auf den Bytecode hat, sind die Felder damit gekennzeichnet, was mich zu der Annahme veranlasst, dass dies ein Nebeneffekt einer JVM-Optimierung ist.
Niv Steingarten
9
@ Eugene: Es sollte nicht enden. Die Frage ist "warum endet es?". A Point pwird konstruiert, was erfüllt p.x+1 == p.y, dann wird eine Referenz an den Abfragethread übergeben. Schließlich entscheidet sich der Abfragethread zum Beenden, weil er der Meinung ist, dass die Bedingung für eines der Pointempfangenen s nicht erfüllt ist , aber dann zeigt die Konsolenausgabe, dass sie erfüllt sein sollte. Das Fehlen von volatilehier bedeutet einfach, dass der Polling-Thread stecken bleiben kann, aber das ist hier eindeutig nicht das Problem.
Erma K. Pizarro
21
@JohnNicholas: Der echte Code (was offensichtlich nicht der Fall ist) hatte eine 100% ige Testabdeckung und Tausende von Tests, von denen viele Dinge in Tausenden von verschiedenen Ordnungen und Permutationen testeten ... Beim Testen wird nicht jeder Randfall auf magische Weise durch Nichtdeterminismus verursacht JIT / Cache / Scheduler. Das eigentliche Problem ist, dass der Entwickler, der diesen Code geschrieben hat, nicht wusste, dass die Konstruktion nicht vor der Verwendung des Objekts erfolgt. Beachten Sie, wie das Entfernen des Leerzeichens dazu führt, synchronizeddass der Fehler nicht auftritt? Das liegt daran, dass ich zufällig Code schreiben musste, bis ich einen fand, der dieses Verhalten deterministisch reproduziert.
Hund

Antworten:

140

Offensichtlich findet das Schreiben in currentPos nicht statt - vor dem Lesen, aber ich sehe nicht, wie das das Problem sein kann.

currentPos = new Point(currentPos.x+1, currentPos.y+1);führt einige Dinge aus, einschließlich des Schreibens von Standardwerten in xund y(0) und des anschließenden Schreibens ihrer Anfangswerte in den Konstruktor. Da Ihr Objekt nicht sicher veröffentlicht wird, können diese 4 Schreibvorgänge vom Compiler / JVM frei neu angeordnet werden.

Aus der Sicht des Lesethreads ist es also eine legale Ausführung, xmit seinem neuen Wert zu lesen , aber beispielsweise ymit seinem Standardwert 0. Wenn Sie die printlnAnweisung erreichen (die übrigens synchronisiert ist und daher die Leseoperationen beeinflusst), haben die Variablen ihre Anfangswerte und das Programm druckt die erwarteten Werte.

Das Markieren currentPosals volatilegewährleistet eine sichere Veröffentlichung, da Ihr Objekt effektiv unveränderlich ist. Wenn das Objekt in Ihrem tatsächlichen Anwendungsfall nach der Erstellung mutiert volatilewird, reichen Garantien nicht aus und Sie können ein inkonsistentes Objekt erneut sehen.

Alternativ können Sie das PointUnveränderliche herstellen, wodurch auch ohne Verwendung eine sichere Veröffentlichung gewährleistet wird volatile. Um Unveränderlichkeit zu erreichen, müssen Sie nur markieren xund yabschließen.

Als Randnotiz und wie bereits erwähnt, synchronized(this) {}kann von der JVM als No-Op behandelt werden (ich verstehe, dass Sie es aufgenommen haben, um das Verhalten zu reproduzieren).

Assylien
quelle
4
Ich bin mir nicht sicher, aber würde es nicht den gleichen Effekt haben, x und y final zu machen und die Speicherbarriere zu umgehen?
Michael Böckling
3
Ein einfacheres Design ist ein unveränderliches Punktobjekt, das Invarianten bei der Konstruktion testet. Sie riskieren also nie, eine gefährliche Konfiguration zu veröffentlichen.
Ron
@BuddyCasino Ja, das habe ich hinzugefügt. Um ehrlich zu sein, erinnere ich mich nicht an die gesamte Diskussion vor 3 Monaten (die Verwendung von final wurde in den Kommentaren vorgeschlagen, daher bin ich mir nicht sicher, warum ich es nicht als Option aufgenommen habe).
Assylias
2
Die Unveränderlichkeit selbst garantiert keine sichere Veröffentlichung (wenn x und y privat wären, aber nur mit Gettern belichtet würden, würde das gleiche Veröffentlichungsproblem weiterhin bestehen). endgültig oder flüchtig garantiert es. Ich würde Final gegenüber Volatile bevorzugen.
Steve Kuo
@SteveKuo Unveränderlichkeit erfordert final - ohne final ist das Beste, was Sie bekommen können, effektive Unveränderlichkeit, die nicht die gleiche Semantik hat.
Assylias
29

Da currentPosaußerhalb des Threads geändert wird, sollte es markiert werden als volatile:

static volatile Point currentPos = new Point(1,2);

Ohne flüchtig kann der Thread nicht garantiert Aktualisierungen von currentPos einlesen, die im Hauptthread vorgenommen werden. Daher werden weiterhin neue Werte für currentPos geschrieben, aber der Thread verwendet aus Leistungsgründen weiterhin die vorherigen zwischengespeicherten Versionen. Da nur ein Thread currentPos ändert, können Sie ohne Sperren davonkommen, was die Leistung verbessert.

Die Ergebnisse sehen sehr unterschiedlich aus, wenn Sie die Werte nur ein einziges Mal innerhalb des Threads lesen, um sie zu vergleichen und anschließend anzuzeigen. Wenn ich dies tue, wird Folgendes ximmer als 1und yzwischen 0einer großen Ganzzahl angezeigt . Ich denke, das Verhalten an dieser Stelle ist ohne das volatileSchlüsselwort etwas undefiniert und es ist möglich, dass die JIT-Kompilierung des Codes dazu beiträgt, dass es sich so verhält. Auch wenn ich den leeren synchronized(this) {}Block auskommentiere, funktioniert der Code ebenfalls und ich vermute, dass das Sperren eine ausreichende Verzögerung verursacht currentPosund die Felder erneut gelesen und nicht aus dem Cache verwendet werden.

int x = p.x + 1;
int y = p.y;

if (x != y) {
    System.out.println(x+" "+y);
    System.exit(1);
}
Ed Plese
quelle
2
Ja, und ich könnte auch einfach alles verschließen. Worauf willst du hinaus?
Hund
Ich habe einige zusätzliche Erklärungen für die Verwendung von hinzugefügt volatile.
Ed Plese
19

Sie haben normalen Speicher, die 'currentpos'-Referenz und das Point-Objekt und seine Felder dahinter, die von zwei Threads ohne Synchronisation gemeinsam genutzt werden. Daher gibt es keine definierte Reihenfolge zwischen den Schreibvorgängen, die in diesem Speicher im Hauptthread ausgeführt werden, und den Lesevorgängen im erstellten Thread (nennen Sie es T).

Der Hauptthread führt die folgenden Schreibvorgänge aus (das Ignorieren der anfänglichen Einrichtung von point führt dazu, dass px und py Standardwerte haben):

  • zu px
  • zu py
  • zu currentpos

Da diese Schreibvorgänge in Bezug auf Synchronisation / Barrieren nichts Besonderes sind, ist die Laufzeit frei, damit der T-Thread sie in beliebiger Reihenfolge sehen kann (der Haupt-Thread sieht natürlich immer Schreib- und Lesevorgänge, die gemäß der Programmreihenfolge sortiert sind) und auftreten zu jedem Zeitpunkt zwischen den Lesevorgängen in T.

Also macht T:

  1. liest currentpos auf p
  2. Lesen Sie px und py (in beliebiger Reihenfolge)
  3. Vergleiche und nimm den Zweig
  4. Lesen Sie px und py (in beliebiger Reihenfolge) und rufen Sie System.out.println auf

Da es keine Ordnungsbeziehungen zwischen den Schreibvorgängen in main und den Lesevorgängen in T gibt, gibt es eindeutig mehrere Möglichkeiten, wie Sie Ihr Ergebnis erzielen können, da T möglicherweise das Schreiben von main in currentpos vor den Schreibvorgängen in currentpos.y oder currentpos.x sieht:

  1. Es liest zuerst currentpos.x, bevor der x-Schreibvorgang stattgefunden hat - erhält 0, dann liest currentpos.y, bevor der y-Schreibvorgang stattgefunden hat - erhält 0. Vergleiche evals mit true. Die Schreibvorgänge werden für T. sichtbar. System.out.println wird aufgerufen.
  2. Es liest zuerst currentpos.x, nachdem das x-Schreiben stattgefunden hat, und liest dann currentpos.y, bevor das y-Schreiben stattgefunden hat - erhält 0. Vergleiche evals mit true. Schriften werden für T ... usw. sichtbar.
  3. Es liest zuerst currentpos.y, bevor der y-Schreibvorgang stattgefunden hat (0), und liest dann currentpos.x, nachdem der x-Schreibvorgang auf true ausgewertet wurde. etc.

und so weiter ... Hier gibt es eine Reihe von Datenrennen.

Ich vermute, dass die fehlerhafte Annahme hier darin besteht, dass die aus dieser Zeile resultierenden Schreibvorgänge über alle Threads in der Programmreihenfolge des Threads sichtbar gemacht werden, der sie ausführt:

currentPos = new Point(currentPos.x+1, currentPos.y+1);

Java gibt keine solche Garantie (es wäre schrecklich für die Leistung). Es muss noch etwas hinzugefügt werden, wenn Ihr Programm eine garantierte Reihenfolge der Schreibvorgänge in Bezug auf Lesevorgänge in anderen Threads benötigt. Andere haben vorgeschlagen, die x-, y-Felder endgültig zu machen oder alternativ currentpos flüchtig zu machen.

  • Wenn Sie die Felder x, y endgültig machen, garantiert Java, dass die Schreibvorgänge ihrer Werte in allen Threads ausgeführt werden, bevor der Konstruktor zurückkehrt. Da die Zuweisung zu currentpos nach dem Konstruktor erfolgt, wird dem T-Thread garantiert, dass die Schreibvorgänge in der richtigen Reihenfolge angezeigt werden.
  • Wenn Sie currentpos flüchtig machen, garantiert Java, dass dies ein Synchronisationspunkt ist, der für andere Synchronisationspunkte insgesamt geordnet ist. Wie in der Regel müssen die Schreibvorgänge in x und y vor dem Schreiben in currentpos erfolgen. Bei jedem Lesen von currentpos in einem anderen Thread müssen auch die zuvor beschriebenen Schreibvorgänge von x, y angezeigt werden.

Die Verwendung von final hat den Vorteil, dass die Felder unveränderlich werden und die Werte zwischengespeichert werden können. Die Verwendung von flüchtigen Daten führt zu einer Synchronisierung bei jedem Schreiben und Lesen von aktuellen Poss, was die Leistung beeinträchtigen kann.

Weitere Informationen finden Sie in Kapitel 17 der Java-Sprachspezifikation: http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html

(Bei der ersten Antwort wurde von einem schwächeren Speichermodell ausgegangen, da ich nicht sicher war, ob die von JLS garantierte Volatilität ausreicht. Die Antwort wurde bearbeitet, um den Kommentar von Assylias widerzuspiegeln ).

paulj
quelle
2
Dies ist meiner Meinung nach die beste Erklärung. Vielen Dank!
Skyde
1
@skyde aber falsch in der Semantik von volatile. volatile garantiert, dass beim Lesen einer flüchtigen Variablen der letzte verfügbare Schreibvorgang einer flüchtigen Variablen sowie alle vorhergehenden Schreibvorgänge angezeigt werden . In diesem Fall currentPosgewährleistet die Zuweisung , wenn sie flüchtig gemacht wird, eine sichere Veröffentlichung des currentPosObjekts sowie seiner Mitglieder, auch wenn sie selbst nicht flüchtig sind.
Assylias
Nun, ich sagte, dass ich selbst nicht genau sehen konnte, wie die JLS garantierte, dass flüchtig eine Barriere mit anderen, normalen Lese- und Schreibvorgängen bildete. Technisch kann ich mich da nicht irren;). Bei Speichermodellen ist es ratsam anzunehmen, dass eine Bestellung nicht garantiert ist und falsch ist (Sie sind immer noch sicher) als umgekehrt und falsch und unsicher. Es ist großartig, wenn Volatile diese Garantie bietet. Können Sie erklären, wie Kapitel 17 des JLS dies bietet?
Paulj
2
Kurz gesagt, Point currentPos = new Point(x, y)Sie haben 3 Schreibvorgänge: (w1) this.x = x, (w2) this.y = yund (w3) currentPos = the new point. Die Programmreihenfolge garantiert, dass hb (w1, w3) und hb (w2, w3). Später im Programm lesen Sie (r1) currentPos. Wenn currentPoses nicht flüchtig ist, gibt es kein hb zwischen r1 und w1, w2, w3, so dass r1 irgendeinen (oder keinen) von ihnen beobachten kann. Mit volatile führen Sie hb (w3, r1) ein. Und die hb-Beziehung ist transitiv, so dass Sie auch hb (w1, r1) und hb (w2, r1) einführen. Dies ist in Java Concurrency in Practice (3.5.3. Safe Publication Idioms) zusammengefasst.
Assylias
2
Ah, wenn hb auf diese Weise transitiv ist, dann ist das eine ausreichend starke 'Barriere', ja. Ich muss sagen, es ist nicht einfach festzustellen, dass 17.4.5 des JLS hb definiert, um diese Eigenschaft zu haben. Es ist sicherlich nicht in der Liste der Eigenschaften zu Beginn des 17.4.5 angegeben. Transitive Schließung wird erst weiter unten nach einigen Erläuterungen erwähnt! Wie auch immer, gut zu wissen, danke für die Antwort! :). Hinweis: Ich werde meine Antwort aktualisieren, um den Kommentar von Assylias widerzuspiegeln.
Paulj
-2

Sie können ein Objekt verwenden, um die Schreib- und Lesevorgänge zu synchronisieren. Andernfalls erfolgt, wie bereits erwähnt, ein Schreiben in currentPos in der Mitte der beiden Lesevorgänge p.x + 1 und py

new Thread() {
    void f(Point p) {
        if (p.x+1 != p.y) {
            System.out.println(p.x+" "+p.y);
            System.exit(1);
        }
    }
    @Override
    public void run() {
        while (currentPos == null);
        while (true)
            f(currentPos);
    }
}.start();
Object sem = new Object();
while (true) {
    synchronized(sem) {
        currentPos = new Point(currentPos.x+1, currentPos.y+1);
    }
}
Germano Fronza
quelle
Eigentlich macht das den Job. Bei meinem ersten Versuch habe ich den Lesevorgang in den synchronisierten Block eingefügt, aber später wurde mir klar, dass dies nicht wirklich notwendig war.
Germano Fronza
1
-1 Die JVM kann beweisen, dass sie semnicht geteilt wird, und die synchronisierte Anweisung als No-Op behandeln ... Die Tatsache, dass sie das Problem löst, ist reines Glück.
Assylias
4
Ich hasse Multithread-Programmierung, viel zu viele Dinge funktionieren aus Glück.
Jonathan Allen
-3

Sie greifen zweimal auf currentPos zu und geben keine Garantie dafür, dass es zwischen diesen beiden Zugriffen nicht aktualisiert wird.

Beispielsweise:

  1. x = 10, y = 11
  2. Worker-Thread wertet px als 10 aus
  3. Der Hauptthread führt das Update aus, jetzt x = 11 und y = 12
  4. Worker-Thread bewertet py als 12
  5. Der Arbeitsthread bemerkt, dass 10 + 1! = 12 ist, druckt und beendet sich.

Sie vergleichen im Wesentlichen zwei verschiedene Punkte.

Beachten Sie, dass selbst wenn Sie currentPos flüchtig machen, Sie nicht davor geschützt sind, da es zwei separate Lesevorgänge durch den Worker-Thread sind.

Fügen Sie eine hinzu

boolean IsValid() { return x+1 == y; }

Methode zu Ihrer Punkteklasse. Dadurch wird sichergestellt, dass bei der Überprüfung von x + 1 == y nur ein Wert von currentPos verwendet wird.

user2686913
quelle
currentPos wird nur einmal gelesen, sein Wert in p kopiert. p wird zweimal gelesen, zeigt aber immer auf dieselbe Stelle.
Jonathan Allen