What exactly are the dangers of passing 'this' from a Java constructor?

Question

So it seems that it's a bad idea to pass this from a constructor in Java.

class Foo {
    Foo() {
        Never.Do(this);
    }
}

My simple question is: Why?

There are some related questions on Stackoverflow, but none of them give a comprehensive list of issues that may arise.

For instance, in this question, which is asking for a workaround to this problem, one of the answers states:

For example if your class has a final (non-static) field, then you can usually depend on it being set to a value and never changing.

When the object you look at is currently executing its constructor, then that guarantee no longer holds true.

How is that?

Also, I understand that subclassing is a big issue, because the superclass constructor is always called before the subclass constructor, and that may cause problems.

Besides, I read that Java Memory Model (JMM) issues such as differences in visibility across threads and memory access reordering may arise, but no details on that.

What other problems may occur and can you elaborate on the abovementioned issues?

Answer 1

Basically, you already enumerate the bad things that can happen, so you partly answer your own question already. I will provide the details for the things you mention:

Handing out this before initializing final fields

For example if your class has a final (non-static) field, then you can usually depend on it being set to a value and never changing.

When the object you look at is currently executing its constructor, then that guarantee no longer holds true.

How is that?

Quite simple: If you hand out this before setting a final field, then it will not be set, yet:

class X{
    final int i;
    X(){
        new Y(this); // ouch, don't do this!
        i = 5;
    }
}

class Y{
    Y(X x){
        assert(x.i == 5);//This assert should be true, since i is final field, but it fails here
    }
}

Quite simple, right? Class Y sees a X with an uninitialized final field. This is a big no-no!

Java usually ensures that a final field is initialized exactly once and is not read before it is initialized. This guarantee vanishes once you leak this.

Note that the same problem also happens for non-final fields which is equally bad. However, people are more surprised if a final field is found uninitialized.

Subclassing

The problems with subclassing are quite similar to the one above: Base classes get initialized before derived classes, so if you leak a this reference in the base class constructor, you leak an object that has not yet initialized its derived fields. This can become very nasty in case of polymorphic methods, as this example shows:

class A{
    static void doFoo(X x){
        x.foo();
    }
}

class X{
    X(){
        A.doFoo(this); // ouch, don't do this!
    }

    void foo(){
        System.out.println("Leaking this seems to work!");
    }

}

class Y extends X {
     PrintStream stream;

     Y(){
         this.stream = System.out;
     }

     @Overload // Polymorphism ruins everything!
     void foo(){
         // NullPointerException; stream not yet initialized 
         stream.println("Leaking + Polymorphism == NPE");      
     }

}

So as you see, there is a class X with a foo method. X is leaked to A in its constructor and A calls foo. For X classes, this works just fine. But for Y classes, a NullPointerException is thrown. The reason is that Y overrides foo and uses one of its fields (stream) in it. Since stream is not initialized yet when A calls foo, you receive the exception.

This example shows the next problem with leaking this: Even if your base class might work just fine when leaking this, a class inheriting from your base class (which may not be written by you but someone else who is not aware of leaked this) may blow everything up.

Leaking this to yourself

This section does not exactly talk about an own kind of problem, but a thing to bear in mind: Even calling one of your own methods can be considered as a leaked this, as it brings similar problems as a reference leaked to another class. For example, consider the previous example with a different X constructor:

    X(){
        // A.doFoo();
        foo(); // ouch, don't do this!
    }

Now, we do not leak this to A but we leak it to ourself by calling foo. Again, the same bad things happen: A class Y that overrides foo() and uses one of its own fields will wreak havok.

Now consider our first example with the final field. Again, leaking to yourself via a method may allow for finding a final field uninitialized:

class X{
   final int i;
   X(){ 
      foo();
      i = 5;
   }

   void foo(){
      assert(i == 5); // Fails, of course
   }
}

Of course, this example is quite constructed. Every programmer will notice that first calling foo and then setting i is wrong. But now consider inheritance again: Your X.foo() method might not even use i, so it is fine to be called before initializing i. However, a subclass might override foo() and use i in it, again breaking everything.

Also note that an overridden foo() method might leak this even further by passing it to other classes. So while we only intended to leak this to ourselves by calling foo() a subclass might override foo() and publish this to the whole world.

Whether calling one's own methods is considered a leaked this might be debatable. However, as you see, it brings similar problems, so I wanted to discuss it here, even if many people may not agree that calling an own method is considered a leaked this.

If you really need to call your own methods in a constructor, then either use only final or static methods, as these cannot be overridden by an innocent derived class.

Concurrency

Final fields in the Java memory model possess a nice property: They can be read concurrently without locking. The JVM has to ensure that even concurrent unlocked access will always see a fully initialized final field. This can be done, by example, by adding a memory barrier to the end of a constructor that assigns final fields. However, this guarantee vanishes once you hand out this too early. Again, an example:

class X{
    final int i;
    X(Y y){
        i = 5;
        y.x = this; // ouch, don't do this!
    }
}

class Y{
    public static Y y;
    public X x;
    Y(){
        new X(this);
    }
}

//... Some code in one thread
{
    Y.y = new Y();
}

//... Some code in another thread
{
    assert(Y.y.x.i == 5); // May fail!
}

As you see, we again hand out this too early, but only after initializing i. So in a single threaded environment, everything is fine. But now enter concurrency: We create a static Y (which receives the tainted X instance) in one thread and access it in a second thread. Now, the assertion may again fail, because the compiler or the CPU out of order execution are now allowed to reorder the assignment of i = 5 and the assignment of Y.y = new Y().

To make things clearer, suppose the JVM would inline all calls, thus, the code

{
        Y.y = new Y();
}

would be first inlined to (rX are local registers):

{
        r1 = 'allocate memory for Y' // Constructor of Y
        r1.x = new X(r1);            // Constructor of Y
        Y.y = r1;
}

now we would also inline the call to new X():

{
        r1 = 'allocate memory for Y' // constructor of Y
        r2 = 'allocate memory for X' // constructor of X
        r2.i = 5;                    // constructor of X
        r1.x = r2;                   // constructor of X
        Y.y = r1;
}

Until now, everything is fine. But now, the reordering is allowed to happen. We (i.e., the JVM or the CPU) reorder r2.i = 5 to the end:

{
        r1 = 'allocate memory for Y' // 1.
        r2 = 'allocate memory for X' // 2.
        r1.x = r2;                   // 3.
        Y.y = r1;                    // 4.
        r2.i = 5;                    // 5.
}

Now, we can observe the wrong behaviour: Consider thread 1 executes all steps up to 4. and then is interrupted (before the final field is set!). Now, thread 2 executes all code and thus its assert(Y.y.x == 5); fails.

What other problems may occur

Basically, the three problems you mentioned and I explained above are the worst ones. Of course, there are many different facets in which these problems can occur so one could construct thousands of examples. As long as your program is single threaded, handing out this early might be okay (but don't do it anyway!). As soon as concurrency comes into play, never ever do it, you will get strange behaviours because the JVM is basically allowed to reorder things as desired in this case. Instead of remembering the gory details of various concrete problems that can happen, simply remember the two conceptual things that can happen:

• A constructor usually first constructs an object thoroughly and then hands it to the caller. A this that escapes the constructor represents a partly constructed object and usually you never ever want to have partly constructed objects, because they are hard to reason about (see my first example). Especially when inheritance comes into play, things get even more complicated. Simply rember: Leaking this + inheritance = Here be dragons.
• The memory model gives up most guarantees once you hand out this in a constructor, so crazy reorderings might yield very strange executions that are almost impossible to debug. Simply rember: Leaking this + concurreny = Here be dragons.