Leaking this in constructor warning

user_21348 0
Viewed 58341

I'd like to avoid (most of the) warnings of Netbeans 6.9.1, and I have a problem with the 'Leaking this in constructor' warning.

I understand the problem, calling a method in the constructor and passing "this" is dangerous, since "this" may not have been fully initialized.

It was easy to fix the warning in my singleton classes, because the constructor is private and only called from the same class.

Old code (simplified):

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

public static Singleton getInstance() {

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

New code (simplified):

private Singleton() {
  ...
}

public static Singleton getInstance() {

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

This fix is not working if the constructor is public and can be called from other classes. How is it possible to fix the following code:

public class MyClass {

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

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

}

Of course I want a fix which does not require to modify all my codes using this class ( by calling an init method for instance).

netbeans-6.9 netbeans this constructor java
11 Answers

The annotation @SuppressWarnings("LeakingThisInConstructor") applicable only to the class an not to the constructor itself.

Solution I would suggest: create private method init(){/* use this here*/} and call it from the constructor. The NetBeans won't warn you.

user_6072050

The error "Leaking this in constructor" is one of the bugs that are really annoying and hard to tell if it is actually a problem. Test cases will pass most of the time and they might even be setup in a way that they pass always and the error only occurs on production.

Having such code with Listeners in the UI is quite common and they usually don't fail because of the single UI thread that makes everything easier, but sometimes not. If you create an object within the UI thread and this object adds some listeners within the UI thread, it is very likely that those listeners are called also within the UI thread since they react to UI events. If you are sure about the setup there, you may just ignore the error.

On the other hand when dealing with multi-threaded applications and classes that for example handle some IO operations and background work, this issue can cause bugs that are hard to detect and usually occur on the weekend when the admin is on holidays and the presentation on the next day includes the biggest clients of the company.

Here is how I would fix the code:

public class MyClass {

    private final List<MyClass> instances = new ArrayList<MyClass>();

    public static MyClass create() {
        MyClass mc = new MyClass();
        mc.init();
        return mc;
    }

    /** private constructor. */
    private MyClass() {}

    private void init() {
        instances.add(this);
    }

}

Provide a factory method or create a separate factory class that is able to create your objects. This factory is responsible to call the init() method after the object has been created.

This requires you to search for references where the constructor is used and update to the new method. As a refactoring step in between I suggest to deprecate the constructor so that you can update the clients to use the new factory method before it is changed, e.g.:

public class MyClass {

    private final List<MyClass> instances = new ArrayList<MyClass>();

    public static MyClass create() {
        MyClass mc = new MyClass();
        // mc.init(); // will be called when the constructor call will be removed
        return mc;
    }

    /** @deprecated use {@link #create()} instead. */
    @Deprecated
    public MyClass() {
        init();
    }

    private void init() {
        instances.add(this);
    }

}

An alternative would be to simply make init() public and force your clients to call it. This gives clients control over the lifecycle of created objects and I would even prefer to do so in cases where init() has to do some actual work, like open or use a connection.

user_35835890
Related