Java Leaking This in Constructor

Java leaking this in constructor

Leaking the this reference in the constructor (not controller) is dangerous, especially in a multithreaded environment. This is because the object is not fully constructed until the constructor call finishes. Leaking this from the constructor thus means that the external world gets access to an object which is not yet fully constructed. This may not necessarily lead to problems in a a single-threaded program (although it is possible, but the problem is much more obvious in this case). But if this is leaked to other threads, they can actually try to do something with the object before its construction is finished, which leads to subtle and hard to find bugs.

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.

Leaking 'this' in constructor warning should apply to final classes as well as open ones?

tl;dr: https://youtrack.jetbrains.com/issue/KT-22044 is a good fit regarding this issue.

I will cite what Intellij IDEAs inspection called "Leaking 'this' in constructor" says about this:

This inspection reports dangerous operations inside constructors including:

• Accessing a non-final property in constructor
• Calling a non-final function in constructor
• Using this as a function argument in a constructor of a non-final class

These operations are dangerous because your class can be inherited, and a derived class is not yet initialized at this moment. Typical example:

abstract class Base {
    val code = calculate()
    abstract fun calculate(): Int
}

class Derived(private val x: Int) : Base() {
    override fun calculate() = x
}

fun testIt() {
    println(Derived(42).code) // Expected: 42, actual: 0
}

I think that there should be warning nonetheless as you were able to access a non-initialized variable. The reason: the compiler already disallows direct access to uninitialized variables, i.e. the following will not compile:

class Demo {
  val some : Int
  init {
    println(some) // Variable 'some' must be initialized

but accessing it indirectly compiles and shows the default value of the variable type:

class Demo2 {
  val some : Int
  val someString : String
  init {
    fun Demo2.indirectSome() = some
    fun Demo2.indirectSomeString() = someString
    println(indirectSome()) // prints 0
    println(indirectSomeString()) // prints null; and yes.. this can lead to NullPointerExceptions

and there we also have a "leak", basically accessing some before it should ;-)

Leaking this (in constructor) to the object itself

When you assign this to the field idPadre it is only possible to access the object if you already have a reference to the object in order to access the field. In other words you haven't leaked anything that hasn't already been leaked by the constructor caller, so to speak.

Leaking this usually means leaking a reference to the object from the constructor before the constructor has finished. The problem with leaking this from the constructor is that fields might not have been initialized properly. This is not an issue in this case since you can only access idPadre once the constructor has finished (unless you already leaked this earlier in the constructor - but then you already leaked this!)

If idPadre was a static field then this would indeed be leaking this since other threads can access static fields at any time.

java - how to fix the leaking this in constructor warning

The constructor will return a reference to the dialog. Have the panel set the variable itself once the instance is created.

public class MyJPanel extends JPanel {
  private JDialog dialog;

  public MyJPanel(Frame aFram) {
    dialog = new MyJDialog(aFrame, true);
  }
}

Also, that code will not work because getThis() is not a static method, thus requires a reference to a MyJPanel instance.

Leaking 'this' in constructor of Oracle's Java Tutorials Code Sample for CustomDialog.java

As suggested in your aforementioned post, you could introduce a static creation method, make the constructor private and move the addXxxListener() calls to the static method.

But I'd leave the code as it is, maybe with an @SuppressWarnings annotation and a warning comment that the addXxxListener() calls should stay at the end of the constructor.

After all, it's a warning on a potential problem where the compiler lacks the intelligence to see that the instance is completely finished at that point (at least, in a single-threaded sense - multi-thread safety is a different concern) and all the other classes getting to see the object prematurely will effectively see a finished version.

Leaking this in constructor

You are passing this to an external class (Controller) when the object hasn't been fully constructed. Controller could then reference your object while its construction hasn't finished.

Most people work around this by using a factory method which creates the object first, then passes this externally.

// private to force clients to use the static factory method
private MessageSelect() {
  initComponents();
  messagesJList.setPrototypeCellValue("xxx");
}

public static MessageSelect createInstance() {
  MessageSelect instance = new MessageSelect();
  instance.controller.addObserver(instance);
  return instance;
}

Take a look at this excellent Brian Goetz article on safe object construction.

---

Related Topics