Leaking this in constructor warning Leaking this in constructor warning java java

Leaking this in constructor warning


Since you make sure to put your instances.add(this) at the end of the constructor you should IMHO be safe to tell the compiler to simply suppress the warning (*). A warning, by its nature, doesn't necessarily mean that there's something wrong, it just requires your attention.

If you know what you're doing you can use a @SuppressWarnings annotation. Like Terrel mentioned in his comments, the following annotation does it as of NetBeans 6.9.1:

@SuppressWarnings("LeakingThisInConstructor")

(*) Update: As Isthar and Sergey pointed out there are cases where "leaking" constructor code can look perfectly safe (as in your question) and yet it is not. Are there more readers that can approve this? I am considering deleting this answer for the mentioned reasons.


[Remark by chiccodoro: An explanation why/when leaking this can cause issues, even if the leaking statement is placed last in the constructor:]

Final field semantics is different from 'normal' field semantics. An example,

We play a network game. Lets make a Game object retrieving data from the network and a Player object that Listens to events from the game to act accordingly. The game object hides all the network details, the player is only interested in events:

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.

Seems all good: access to the list is correctly synchronized. The flaw is that this example leaks the Player.this to Game, which is running a thread.

Final is quite scary:

...compilers have a great deal of freedom to move reads of final fields across synchronization barriers...

This pretty much defeats all proper synchronizing. But fortunately

A thread that can only see a reference to an object after that object has been completely initialized is guaranteed to see the correctly initialized values for that object's final fields.

In the example, the constructor writes the objects reference to the list. (And thus has not been completely initialized yet, since the constructor did not finish.) After the write, the constructor is still not done. It just has to return from the constructor, but let's assume it hasn't yet. Now the executor could do its job and broadcast events to all the listeners, including the not yet initialized player object! The final field of the player (name) may not be written, and will result in printing null sees event!.


The best options you have :

  • Extract your WindowFocusListener part in another class (could also be inner or anonymous) . The best solution, this way each class has a specific purpose.
  • Ignore the warning message.

Using a singleton as a workaround for a leaky constructor is not really efficient.