CODEWITHSUNDEEP
javainner-classes
Leem.fin
Leem.fin

Reputation: 42592

create inner class instance in constructor

I am reading the book Java concurrency in practice, in section 3.2 , it gives the following code example to illustrate implicitly allowing the this reference to escape (Don’t do this, especailly in constructor) :

public class ThisEscape {
    public ThisEscape(EventSource source) {
       source.registerListener ( 
             new EventListener() {
                 public void onEvent(Event e) {
                     doSomething(e);
                 }
             }
       );
  } 
}

The book then says :

When ThisEscape publishes the EventListener, it implicitly publishes the enclosing ThisEscape instance as well, because inner class instances contain a hidden reference to the enclosing instance.

I understand the above words from Java's perspective, but I can't come up with a example how could the above code's EventListener escaping enclosing reference this be harmful? In what way?

For example, if I create a new instance of ThisEscape:

ThisEscape myEscape = new Escape(mySource);

Then, what? How is it harmful now? In which way it is harmful?

Could someone please use above code as the base and explain to me how it is harmful?

======= MORE ======

The book is trying to say something like the anonymous EventListener holds a hidden reference to the containing class instance which is not yet fully constructed. I want to know in example, how could this un-fully constructed reference be misused, and I prefer to see a code example about this point.

The book gives a right way of doing things, that's to use a static factory method as below:

public static SafeListener newInstance(EventSource source) { 
       SafeListener safe = new SafeListener(); 
       source.registerListener (safe.listener);
       return safe;
}

I just don't get the point of the whole thing.

Upvotes: 3

Views: 178

Answers (2)

ruakh
ruakh

Reputation: 183251

The issue with publishing an object mid-construction, in a multithreaded context, is that the object may be used before construction completes (or after the constructor raised an exception).

Even if the publishing happens as the last explicit step in the constructor, there are three things to keep in mind:

  • The order of side effects within a thread does not determine the order in which those side effects become visible to other threads. So even if the constructor is written in such a way that it fully populates the object before it publishes a reference to it, there's no guarantee that other threads will see the fully populated object when they read the reference.

  • A final field normally has special concurrency properties, but these properties depend on reaching the end of the constructor before the object becomes visible to other threads. If other threads perceive the object before it's fully constructed, then they may not even see the correct values of final fields.

  • Superclass constructors are called before any initialization happens in a subclass. So, for example, if a subclass contains the field String foo = "foo", then during the superclass constructor, the field will still be null, which will affect the results of virtual methods that use it. So if a reference to the object is published during the superclass constructor, other threads can act on the object while it's in an incomplete (and bizarre) state.

Upvotes: 2

Alex Shesterov
Alex Shesterov

Reputation: 27525

Problem 1: Operating on non-fully-constructed object

Consider this slightly modified example: 

public class ThisEscape {
    private String prefixText = null;

    private void doSomething(Event e) {
        System.out.println(prefixText.toUpperCase() + e.toString());
    }

    public ThisEscape(EventSource source) {
       source.registerListener( 
             new EventListener() {
                 public void onEvent(Event e) {
                     doSomething(e); // hidden reference to `ThisEscape` is used
                 }
             }
       );

       // What if an event is fired at this point from another thread? 
       // prefixText is not yet assigned, 
       // and doSomething() relies on it being not-null

       prefixText = "Received event: ";
    }
}

This would introduce a subtle and very hard-to-find bug, for example in multithreaded applications.

Consider that the event source fires and event after source.registerListener(...) has completed, but before prefixText was assigned. This could happen in a different thread.

In this case, the doSomething() would access the non-yet-initialized prefixText field, which would result in a NullPointerException. In other scenarios, the result could be invalid behavior or wrong calculation results, which would be event worse than an exception. And this kind of error is extremely hard to find in real-world applications, mostly due to the fact that it happens sporadically.

Problem 2: Garbage collection

The hidden reference to the enclosing instance would hinder the garbage collector from cleaning up the "enclosing instance" in certain cases.

This would happen if the enclosing instance isn't needed anymore by the program logic, but the instance of the inner class it produced is still needed.

If the "enclosing instance" in turn holds references to a lot of other objects which aren't needed by the program logic, then it would result in a massive memory leak.

A code example. Given a slightly modified ThisEscape class form the question: 

public class ThisEscape {

    private long[] aVeryBigArray = new long[4711 * 815];

    public ThisEscape(EventSource source) {
       source.registerListener( 
             new EventListener() {
                 public void onEvent(Event e) {
                     doSomething(e);
                 }
                 private void doSomething(Event e) {
                     System.out.println(e.toString());
                 }
             }
       );
    } 
}

Please note that the inner anonymous class (which extends/implements EventListener) is non-static and thus contains a hidden reference to the instance of the containing class (ThisEscape).

Also note that the anonymous class doesn't actually use this hidden reference: no non-static methods or fields from the containing class are used from within the anonymous class.

Now this could be a possible usage: 

// Register an event listener to print the event to System.out
new ThisEscape(myEventSource);

With this code we wanted to achieve that an event is registered within myEventSource. We do not need the instance of ThisEscape anymore.

But assuming that the EventSource.registerListener(EventListener) method stores a reference to the event listener created within ThisEscape, and the anonymous event listener holds a hidden reference to the containing class instance, the instance of ThisEscape can't be garbage-collected.

I've intentionally put a big non-static long array into ThisEscape, to demonstrate that the ThisEscape class instance could actually hold a lot of data (directly or indirectly), so the memory leak can be significant.

Upvotes: 5

Related Questions