How to avoid circular references with the observer pattern?

Question

I've got a situation like this:

public class A
{
    private ArrayList<Listener> listeners = new ArrayList<Listener>();

    public A()
    {
        // stub
    }

    public void addListener(Listener listener)
    {
        this.listeners.add(listener);
    }

    public void removeListener(Listener listener)
    {
        this.listeners.remove(listener);
    }

    public interface Listener
    {
        // stub
    }
}

public class B implements A.Listener
{
    private A instanceOfA;

    public B()
    {
        this.instanceOfA = new A();
        this.instanceOfA.addListener(this);
    }
}

I believe that B can never be destroyed because A is keeping a reference to it, and A can never be destroyed because B is keeping a reference to it.

This seems like a common design pattern, but nobody explains how to avoid this circular reference?

Answer 1

In practice this is solved so that the observer B does not refer to A directly. It should be a small class focused on the sole task of handling its events and shouldn't have any direct coupling to A. In fact, that is the very point of the observer pattern: a flexible way to add code to A without a hard dependency on it.

If the event handler must observe the object which is the event source (A), then the event callback method should declare an A-typed argument so it can be passed to the handler in a stateless manner, to be used only during event handling and not retained afterwards.

Answer 2

Because you are not doing it as the pattern defines it normally. You can take a look at this Tutorial.

Define a Listener as an Interface like this for example:

/**
 * The listener interface for receiving message events. The class that is interested in processing a
 * message event implements this interface.
 *
 * @see MessageEvent
 */
public interface MessageListener
{

  /**
   * Message received notification.
   *
   * @param data the data
   * @throws IOException Signals that an I/O exception has occurred.
   */
  public void MessageReceived( byte[] data ) throws IOException;

  /**
   * Connection closed notification.
   */
  public void closed();
}

Now you can create as much implementations as you need for example like this:

public class MessageListenerImpl implements MessageListener
{

  /** The driver. */
  private DeviceClassInternal driver;

  /** The log. */
  private Log log;

  /**
   * Instantiates a new TCP message listener.
   *
   * @param driver the driver
   * @param log the log
   */
  public MessageListenerImpl( DeviceClassInternal driver, Log log )
  {
    this.driver = driver;
    this.log = log;
  }

  /** {@inheritDoc} */
  @Override
  public void MessageReceived( byte[] data ) throws IOException
  {
    log.info( "data received: {1}", new String( data ) );
    driver.dataReceived( data );
  }

  @Override
  public void closed()
  {
    log.info( "{0}: Connection closed", physicalConnection.getName() );
    driver.closed();
  }

}

and finally you can create it and register this listener to the object you want:

  MessageListenerImpl listener = new MessageListenerImpl( connection, log ); // create the listener and you're good to go.

  physicalConnection.registerListener( listener ); // Just some object with a register function.

Answer 3

It all depends on specific use cases. As other people pointed, in many cases you don't need to hold A reference in your listener and instead rely on having it being passed in the event itself. But this is not always enough - for example, you might want to deregister yourself depending on some other event and you need original reference to call removeListener.

But this is more generic question - if you still hold reference to A somewhere else, it won't get GCed regardless of your listener design. On the other hand, if you have forgotten references to A everywhere in your program and don't hold reference to B as well, they will be both garbage collected anyway - circular references are not preventing garbage collection in java (How does Java Garbage Collection work with Circular References?)

I had bit similar situation in the past, but I cared about B being GCed rather than A. When all references to B from outside are lost, there is nobody to unregister it, but it is still getting notified from A on every event. It is quite common to end with such danging listeners in UI frameworks in java if you don't clean up after yourself perfectly (which people often don't do, as they assume that getting rid of the UI component graphically and forgetting all references to it should be enough - while for example, some listener to global keyboard handler or something still holds everything in strongly reachable set).

While for UI frameworks you don't get a chance to change core code, with your own code you can try to make listener lists having WeakReference instead of strong references and do some cleanup on every notification if needed. It has a side effect of you having to keep your listeners references from other places of code, but it is a good practice anyway (as you should be deregistering them manually in other case) - if you don't do that, with weak reference you would suddenly stop getting called back at random times (after few gc cycles have run).

In any case, you first need to understand (and tell us), how do you imagine lifecycle of A and B and why B will be strongly referenced after A could already disappear. You can always make B->A reference weak as well, but please first be sure to fully understand your expectations about what gets forgotten at what point.