Getting Java Event Notification Right

Implementing the observer pattern to provide Java event notification seems to be a straight forward thing to do. However there are some pitfalls one easily can run into. Here comes an explanation of common mistakes I carelessly have produced myself on various occasions…

Java Event Notification

Let’s start with a simple bean StateHolder that encapsulates a private int field state with appropriate accessors:
 
 

public class StateHolder {

  private int state;

  public int getState() {
    return state;
  }

  public void setState( int state ) {
    this.state = state;
  }
}

Consider that we have decided our bean should broadcast the news of state changes to registered observers. No problem at all! A convenient event and listener definition is easy to create…

// change event to broadcast
public class StateEvent {

  public final int oldState;
  public final int newState;

  StateEvent( int oldState, int newState ) {
    this.oldState = oldState;
    this.newState = newState;
  }
}

// observer interface
public interface StateListener {
  void stateChanged( StateEvent event );
}

…next we need to be able to register StatListeners at StateHolder instances…

public class StateHolder {

  private final Set<StateListener> listeners = new HashSet<>();

  [...]
     
  public void addStateListener( StateListener listener ) {
    listeners.add( listener );
  }

  public void removeStateListener( StateListener listener ) {
    listeners.remove( listener );
  }
}

… and last but not least StateHolder#setState have to be adjusted to trigger the actual notification on state changes:

public void setState( int state ) {
  int oldState = this.state;
  this.state = state;
  if( oldState != state ) {
    broadcast( new StateEvent( oldState, state ) );
  }
}

private void broadcast( StateEvent stateEvent ) {
  for( StateListener listener : listeners ) {
    listener.stateChanged( stateEvent );
  }
}

Bingo! That’s all there is. Being professionals we might even have implemented this test driven and feel comfortable with our thorough code coverage and the green bar. And anyway isn’t this what we have learned from tutorials on the web?

So here comes the bad news: The solution is flawed…

Concurrent Modification

Given the StateHolder above one can easily run into a ConcurrentModificationException, even if used within single thread confinement only. But who is causing it and why does it occur?

java.util.ConcurrentModificationException
	at java.util.HashMap$HashIterator.nextNode(HashMap.java:1429)
	at java.util.HashMap$KeyIterator.next(HashMap.java:1453)
	at com.codeaffine.events.StateProvider.broadcast(StateProvider.java:60)
	at com.codeaffine.events.StateProvider.setState(StateProvider.java:55)
	at com.codeaffine.events.StateProvider.main(StateProvider.java:122)

A look at the stacktrace uncovers that the exception is thrown by an Iterator of the HashMap we use. Only that we did not use any iterators in our code, or did we? Well, we did. The for each construct in broadcast is based on Iterable and therefore gets transformed into an iterator loop at compile time.

Because of this a listener removing itself from the StateHolder instance during event notification might cause the ConcurrentModificationException. So instead of working on the original data structure one solution could be to iterate over a snapshot of listeners.

By doing so listener removal cannot interfere with the broadcast mechanism anymore (but note that notification semantics changes also slightly, since such a removal is not reflected by the snapshot while broadcast gets executed):

private void broadcast( StateEvent stateEvent ) {
  Set<StateListener> snapshot = new HashSet<>( listeners );
  for( StateListener listener : snapshot ) {
    listener.stateChanged( stateEvent );
  }
}

But what if StateHolder is meant to be used within a multi threaded context?

Synchronization

To be able to use StateHolder within a multi threaded environment it has to be thread safe. This can be achieved quite easily. Adding synchronized to each method of our class should do the trick, right?

public class StateHolder {
  public synchronized void addStateListener( StateListener listener ) {  [...]
  public synchronized void removeStateListener( StateListener listener ) {  [...]
  public synchronized int getState() {  [...]
  public synchronized void setState( int state ) {  [...]

Now read/write access to a StateHolder instance is guarded by its intrinsic lock. This makes the public methods atomic and ensures correct state visibility for different threads. Mission accomplished!

Not quite… although the implementation is thread safe, it bears the risk to dead lock applications that use it.

Think about the following situation: Thread A changes the state of StateHolder S. During the notification of the listeners of S Thread B tries to access S and gets blocked. If B holds a synchronization lock on an object that is about to be notified by one of the listeners of S, we run into a dead lock.

That is why we need to narrow down synchronization to state access and broadcast the event outside of the guarded passages:

public class StateHolder {

  private final Set<StateListener> listeners = new HashSet<>();
  private int state;

  public void addStateListener( StateListener listener ) {
    synchronized( listeners ) {
      listeners.add( listener );
    }
  }

  public void removeStateListener( StateListener listener ) {
    synchronized( listeners ) {
      listeners.remove( listener );
    }
  }

  public int getState() {
    synchronized( listeners ) {
      return state;
    }
  }

  public void setState( int state ) {
    int oldState = this.state;
    synchronized( listeners ) {
      this.state = state;
    }
    if( oldState != state ) {
      broadcast( new StateEvent( oldState, state ) );
    }
  }

  private void broadcast( StateEvent stateEvent ) {
    Set<StateListener> snapshot;
    synchronized( listeners ) {
      snapshot = new HashSet<>( listeners );
    }
    for( StateListener listener : snapshot ) {
      listener.stateChanged( stateEvent );
    }
  }
}

The listing shows an implementation evolved from the previous snippets providing proper (but somewhat old fashioned) synchronization using the Set instance as internal lock. Listener notification takes place outside of the guarded block and therefore avoids a circular wait.

Note: Due to the concurrent nature of the system, the solution does not guarantee that change notifications reach a listener in the order they occured. If more accuracy about the actual state value on observer side is needed, consider to provide the StateHolder as source of your event object.

If event ordering is crucial one could think of a thread safe FIFO structure to buffer events along with the according listener snapshot in the guarded block of setState. A separate thread could fire the actual event notifications from an unguarded block as long as the FIFO structure is not empty (Producer-Consumer-Pattern). This should ensure chronological order without risking a dead lock. I say should since I never tried this solution by myself..

Given the sematics of the previous implementation, composing our class using thread safe classes like CopyOnWriteArraySet and AtomicInteger makes the solution less verbose:

public class StateHolder {

  private final Set<StateListener> listeners = new CopyOnWriteArraySet<>();
  private final AtomicInteger state = new AtomicInteger();

  public void addStateListener( StateListener listener ) {
    listeners.add( listener );
  }

  public void removeStateListener( StateListener listener ) {
    listeners.remove( listener );
  }

  public int getState() {
    return state.get();
  }

  public void setState( int state ) {
    int oldState = this.state.getAndSet( state );
    if( oldState != state ) {
      broadcast( new StateEvent( oldState, state ) );
    }
  }

  private void broadcast( StateEvent stateEvent ) {
    for( StateListener listener : listeners ) {
      listener.stateChanged( stateEvent );
    }
  }
}

Since CopyOnWriteArraySet and AtomicInteger are thread safe we do not have the need for guarded blocks anymore. But wait a moment! Didn’t we just learn to use a snapshot for broadcasting instead of looping over an hidden iterator of the origin set?

It might be a bit confusing, but an Iterator provided by CopyOnWriteArraySet is already a snapshot. CopyOnWriteXXX collections were invented particularly for such use cases – efficient if small sized, optimized for frequent iteration with rarely changing content. Which means our code is safe.

With Java 8 the broadcast method could be stripped down even more using Iterable#forEach in conjunction with lambdas. The code of course stays safe as iteration is also performed on a snapshot:

private void broadcast( StateEvent stateEvent ) {
  listeners.forEach( listener -> listener.stateChanged( stateEvent ) );
}

Exception Handling

The last section of this post discusses how to handle broken listeners that throw unexpected RuntimeExceptions. Although I usually opt strictly for a fail-fast approach, it might be unappropriate in this case to let such exceptions pass unhandled. Given in particular that the implementation is probably used in a multi threading environment.

A broken listener harms the system in two ways. First, it prevents notification of those observers assorted after our bogey. Second, it can harm the calling thread which may not be prepared to deal with the problem. Summarized it can lead to multiple, sneaking malfunctions of which the initial cause might be hard to track down.

Hence it might be useful to shield each notification within a try-catch block:

private void broadcast( StateEvent stateEvent ) {
  listeners.forEach( listener -> notifySafely( stateEvent, listener ) );
}

private void notifySafely( StateEvent stateEvent, StateListener listener ) {
  try {
    listener.stateChanged( stateEvent );
  } catch( RuntimeException unexpected ) {
    // appropriate exception handling goes here...
  }
}

Conclusion

As shown in the sections above Java event notification has a few nuts and bolts one has to keep in mind. Ensure to iterate over a snapshot of the listener collection during event notification, keep event notification out of synchronized blocks and notify listeners safely if appropriate.

Hopefully I was able to work out the subtleties in a comprehensible way and did not mess up in particular the concurrency sections. In case you find some mistakes or have additional wisdoms to share, feel free to use the commenting sections below.

Do you want to know how to develop your skillset to become a Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

To get you started we give you our best selling eBooks for FREE!

1. JPA Mini Book

2. JVM Troubleshooting Guide

3. JUnit Tutorial for Unit Testing

4. Java Annotations Tutorial

5. Java Interview Questions

6. Spring Interview Questions

7. Android UI Design

and many more ....

I agree to the Terms and Privacy Policy

Sign up

Thank you!

We will contact you soon.