wake-up-neo.com

Durchsickern in Konstruktor-Warnung

Ich möchte (die meisten) Warnungen von Netbeans 6.9.1 vermeiden und habe ein Problem mit der Warnung 'Leaking this in constructor'.

Ich verstehe das Problem. Das Aufrufen einer Methode im Konstruktor und das Übergeben von "this" ist gefährlich, da "this" möglicherweise nicht vollständig initialisiert wurde.

Die Warnung in meinen Singleton-Klassen konnte leicht behoben werden, da der Konstruktor privat ist und nur von derselben Klasse aufgerufen wird.

Alter Code (vereinfacht):

private Singleton() {
  ...
  addWindowFocusListener(this);
}

public static Singleton getInstance() {

  ...
  instance = new Singleton();
  ...
}

Neuer Code (vereinfacht):

private Singleton() {
  ...
}

public static Singleton getInstance() {

  ...
  instance = new Singleton();
  addWindowFocusListener( instance );
  ...
}

Dieser Fix funktioniert nicht, wenn der Konstruktor öffentlich ist und von anderen Klassen aufgerufen werden kann. Wie ist es möglich, den folgenden Code zu korrigieren:

public class MyClass {

  ...
  List<MyClass> instances = new ArrayList<MyClass>();
  ...

  public MyClass() {
    ...
    instances.add(this);
  }

}

Natürlich möchte ich ein Update, bei dem nicht alle meine Codes mit dieser Klasse geändert werden müssen (beispielsweise durch Aufrufen einer init-Methode).

75
asalamon74

Da stellen Sie sicher, dass Sie Ihre instances.add(this) am Ende des Konstruktors setzen sollte IMHO sicher sein, dass der Compiler die Warnung einfach unterdrückt (*) . Eine Warnung bedeutet von Natur aus nicht unbedingt, dass etwas nicht stimmt, es erfordert lediglich Ihre Aufmerksamkeit.

Wenn Sie wissen, was Sie tun, können Sie eine @SuppressWarnings-Anmerkung verwenden. Wie Terrel in seinen Kommentaren erwähnt, macht die folgende Anmerkung dies ab NetBeans 6.9.1:

@SuppressWarnings("LeakingThisInConstructor")

(*) Update: Wie Isthar und Sergey hervorgehoben haben, gibt es Fälle, in denen "undichter" Konstruktorcode absolut sicher aussehen kann (wie in Ihrer Frage) und dennoch nicht. Gibt es mehr Leser, die das genehmigen können? Ich erwäge, diese Antwort aus den genannten Gründen zu löschen.

43
chiccodoro

[Bemerkung von chiccodoro: Eine Erklärung, warum/wenn this ausläuft, kann zu Problemen führen, selbst wenn die undichte Anweisung im Konstruktor als letztes platziert wird:]

Die endgültige Feldsemantik unterscheidet sich von der "normalen" Feldsemantik. Ein Beispiel, 

Wir spielen ein Netzwerkspiel. Lässt ein Game-Objekt Daten aus dem Netzwerk abrufen und ein Player-Objekt, das Ereignisse aus dem Spiel abhört, um entsprechend zu handeln. Das Spielobjekt blendet alle Netzwerkdetails aus, der Spieler ist nur an Ereignissen interessiert:

import Java.util.*;
import Java.util.concurrent.Executors;

public class FinalSemantics {

    public interface Listener {
        public void someEvent();
    }

    public static class Player implements Listener {
        final String name;

        public Player(Game game) {
            name = "Player "+System.currentTimeMillis();
            game.addListener(this);//Warning leaking 'this'!
        }

        @Override
        public void someEvent() {
            System.out.println(name+" sees event!");
        }
    }

    public static class Game {
        private List<Listener> listeners;

        public Game() {
            listeners = new ArrayList<Listener>();
        }

        public void start() {
            Executors.newFixedThreadPool(1).execute(new Runnable(){

                @Override
                public void run() {
                    for(;;) {
                        try {
                            //Listen to game server over network
                            Thread.sleep(1000); //<- think blocking read

                            synchronized (Game.this) {
                                for (Listener l : listeners) {
                                    l.someEvent();
                                }
                            }
                        } catch (InterruptedException e) {
                            e.printStackTrace();
                        }
                    }
                }            
            });
        }

        public synchronized void addListener(Listener l) {
            listeners.add(l);
        }
    }

    public static void main(String[] args) throws InterruptedException {
        Game game = new Game();
        game.start();
        Thread.sleep(1000);
        //Someone joins the game
        new Player(game);
    }
}
//Code runs, won't terminate and will probably never show the flaw.

Scheint alles gut: Der Zugriff auf die Liste ist korrekt synchronisiert. Der Fehler ist, dass dieses Beispiel das Player.this an Game übergibt, in dem ein Thread ausgeführt wird. 

Das Finale ist ziemlich beängstigend :

... Compiler haben große Freiheit, Lesevorgänge der letzten Felder über Synchronisationsbarrieren zu verschieben ...

Dies besiegt ziemlich alle richtige Synchronisation. Aber glücklicherweise 

Ein Thread, der nur einen Verweis auf ein Objekt anzeigen kann, nachdem das Objekt vollständig initialisiert wurde, erkennt garantiert die korrekt initialisierten Werte für die final-Felder dieses Objekts. 

Im Beispiel schreibt der Konstruktor die Objektreferenz in die Liste. (Und wurde daher noch nicht vollständig initialisiert, da der Konstruktor nicht fertiggestellt wurde.) Nach dem Schreiben ist der Konstruktor immer noch nicht fertig. Es muss nur vom Konstruktor zurückkehren, aber nehmen wir an, es ist noch nicht geschehen. Nun konnte der Executor seinen Job erledigen und Ereignisse an alle Zuhörer senden, einschließlich des noch nicht initialisierten Spielerobjekts! Das letzte Feld des Players (Name) wird möglicherweise nicht geschrieben und führt zum Drucken von null sees event!

34
Ishtar

Die besten Optionen, die Sie haben:

  • Extrahieren Sie Ihren WindowFocusListener-Teil in einer anderen Klasse (kann auch inner oder anonym sein). Die beste Lösung, auf diese Weise hat jede Klasse einen bestimmten Zweck.
  • Ignorieren Sie die Warnmeldung.

Ein Singleton als Workaround für einen undichten Konstruktor zu verwenden ist nicht wirklich effizient.

13
Colin Hebert

Dies ist ein guter Fall, in dem eine Factory, die Instanzen Ihrer Klasse erstellt hat, hilfreich wäre. Wenn eine Factory für die Erstellung von Instanzen Ihrer Klasse verantwortlich wäre, hätten Sie einen zentralen Speicherort, an dem der Konstruktor aufgerufen wird, und es wäre einfach, dem Code eine erforderliche init()-Methode hinzuzufügen.

In Bezug auf Ihre unmittelbare Lösung würde ich vorschlagen, dass Sie alle Aufrufe, die this durchgesickert sind, in die letzte Zeile Ihres Konstruktors verschieben und diese dann mit einer Anmerkung unterdrücken, sobald Sie "bewiesen" haben, dass dies sicher ist.

In IntelliJ IDEA können Sie diese Warnung mit dem folgenden Kommentar direkt über der Zeile unterdrücken:
//noinspection ThisEscapedInObjectConstruction 

12
Nate W.

Man kann schreiben: 

addWindowFocusListener(Singleton.this);

Dadurch wird verhindert, dass NB die Warnung anzeigt.

4
Andrew

Die Verwendung einer verschachtelten Klasse (wie von Colin vorgeschlagen) ist wahrscheinlich die beste Option. Hier ist der Pseudocode:

private Singleton() {
  ...
}

public static Singleton getInstance() {

  ...
  instance = new Singleton();
  addWindowFocusListener( new MyListener() );
  ...

  private class MyListener implements WindowFocusListener {
  ...
  }
}
2
Ron Stern

Es ist keine separate Listener-Klasse erforderlich.

public class Singleton implements WindowFocusListener {

    private Singleton() {
      ...
    }    

    private void init() {
      addWindowFocusListener(this);
    }

    public static Singleton getInstance() {    
      ...
      if(instance != null) {
        instance = new Singleton();
        instance.init();
      }
      ...
    }
}
2
jkuruvila

Die Annotation @SuppressWarnings ("LeakingThisInConstructor") gilt nur für die Klasse und nicht für den Konstruktor.

Lösung Ich würde vorschlagen: Private Methode erstellen init () {/ * verwende das hier * /} und rufe es vom Konstruktor aus auf. Die NetBeans werden Sie nicht warnen.

1
CAB

Angenommen, Sie hatten ursprünglich eine solche Klasse, die sich selbst als ActionListener verwendete, und deshalb rufen Sie addActionListener (this) auf, der die Warnung generiert.

private class CloseWindow extends JFrame implements ActionListener {
    public CloseWindow(String e) {
        setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
        setLayout(new BorderLayout());

        JButton exitButton = new JButton("Close");
        exitButton.addActionListener(this);
        add(exitButton, BorderLayout.SOUTH);
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        String actionCommand = e.getActionCommand();

        if(actionCommand.equals("Close")) {
            dispose();
        }
    }
}

Wie @Colin Hebert erwähnt, können Sie den ActionListener in seine eigene Klasse unterteilen. Dies würde natürlich einen Verweis auf den JFrame erfordern, auf dem Sie .dispose () aufrufen möchten. Wenn Sie den Variablennamensraum nicht auffüllen möchten und ActionListener für mehrere JFrames verwenden möchten, können Sie dies mit getSource () tun, um die Schaltfläche abzurufen, gefolgt von einer Kette von getParent () - Aufrufen Rufen Sie die Klasse ab, die JFrame erweitert, und rufen Sie dann getSuperclass auf, um sicherzustellen, dass es sich um einen JFrame handelt.

private class CloseWindow extends JFrame {
    public CloseWindow(String e) {
        setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
        setLayout(new BorderLayout());

        JButton exitButton = new JButton("Close");
        exitButton.addActionListener(new ExitListener());
        add(exitButton, BorderLayout.SOUTH);
    }
}

private class ExitListener implements ActionListener {
    @Override
    public void actionPerformed(ActionEvent e) {
        String actionCommand = e.getActionCommand();
        JButton sourceButton = (JButton)e.getSource();
        Component frameCheck = sourceButton;
        int i = 0;            
        String frameTest = "null";
        Class<?> c;
        while(!frameTest.equals("javax.swing.JFrame")) {
            frameCheck = frameCheck.getParent();
            c = frameCheck.getClass();
            frameTest = c.getSuperclass().getName().toString();
        }
        JFrame frame = (JFrame)frameCheck;

        if(actionCommand.equals("Close")) {
            frame.dispose();
        }
    }
}

Der obige Code funktioniert für jede Schaltfläche, die ein Kind auf einer beliebigen Klasse einer Klasse ist, die JFrame erweitert. Wenn es sich bei Ihrem Objekt nur um einen JFrame handelt, müssen Sie diese Klasse direkt überprüfen, nicht die Superklasse.

Wenn Sie diese Methode verwenden, erhalten Sie letztendlich einen Verweis auf Folgendes: MainClass $ CloseWindow, das über die Superklasse JFrame verfügt, und dann setzen Sie diesen Verweis auf JFrame und entsorgen ihn.

0
sage88

Wickeln Sie Ihre this in doppelte Klammern. Netbeans ignoriert standardmäßig einige Fehler, wenn sie in Unteranweisungen enthalten sind. 

  public MyClass() {
     ...
     instances.add((this));
  }

https://stackoverflow.com/a/8357990

0
user7868