4

I am working on some project in which I am using Observer design Pattern. Subject class is :

public class Subject {
    private List<Observer> observers = new ArrayList<Observer>();
    private int state;
    public void setState(int state) {
          this.state = state;
          notifyAllObservers();
    }
    public void attach(Observer observer){
          observers.add(observer);      
    }
    public void notifyAllObservers(){
          for (Observer observer : observers) {
             observer.update();
           }
   }
   public void deattach(Observer observer) {
         observers.remove(observer);
   }
}

Observer Interface is:

public abstract class Observer implements Runnable{
    protected Subject subject;
    public abstract void update();
    public abstract void process();
}

One of the Observer Named as HexObserver is:

public class HexaObserver extends Observer {

    private ExecutorService threadpool = Executors.newFixedThreadPool(10);

    public HexaObserver(Subject subject) {
        this.subject = subject;
        this.subject.attach(this);
    }

    @Override
    public void update() {
        System.out.println("Hex String: "
                + Integer.toHexString(subject.getState()));
        Future future = threadpool.submit(new HexaObserver(subject));

    }

    @Override
    public void run() {
        // TODO Auto-generated method stub
        System.out.println("In run :D :D :D");

    }

    @Override
        public void process() {
            // TODO 
        }
}

Class to test this is:

public class Demo {
    public static void main(String[] args) {
        Subject subject = new Subject();
        HexaObserver hob = new HexaObserver(subject);
        System.out.println("First state change: 15");
        subject.setState(15);
    }
}

When I tried to run this this is giving some Error:

First state change: 15
Hex String: f
Exception in thread "main" java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:859)
    at java.util.ArrayList$Itr.next(ArrayList.java:831)
    at Observer.Subject.notifyAllObservers(Subject.java:23)
    at Observer.Subject.setState(Subject.java:15)
    at Observer.Demo.main(Demo.java:12)
In run :D :D :D

I didn't get why I am getting this error as ConcurrentModificationException is thrown out when we try to modify some Object concurrently when it is not permissible.

Am I missing something ?

Geek_To_Learn
  • 1,816
  • 5
  • 28
  • 48
  • 2
    It looks like this is because your HexaObserver is adding an element to the array list while the notification loop is happening. Try changing `ArrayList` to a concurrent implementation such as a `CopyOnWriteArrayList` – BretC May 06 '15 at 11:15
  • BretC seems to be right. I dont understand why in update() a new HexaObserver is added (including implicit attach in Constructor). Probably `submit(this)` was intended? Or submit a new immutable and threadsafe Object without implicit attach. – Markus Kull May 06 '15 at 11:25
  • @Bret C : yaa [CopyOnWriterArrayList] will work but It is so costly. By the way method suggested by Markus Kull is also good. I really intended this only. – Geek_To_Learn May 06 '15 at 11:47

2 Answers2

1

Two things are happening at the same time: you are iterating over observers and adding an element to observers. That causes your ConcurrentModificationException

In general there are at least three things you can do:

  • use a synchronized collection
  • thread-safely copy the collection and iterate on a copy
  • manually synchronize all access to observers with synchronized block:

 

public void attach(Observer observer){
      synchronized(observers){ 
          observers.add(observer);      
      }
}
public void notifyAllObservers(){
      synchronized(observers){ 
          for (Observer observer : observers) {
             observer.update();
          }
      }
}
public void deattach(Observer observer) {
      synchronized(observers){ 
           observers.remove(observer);
      }
}
  • you can also mark whole methods as synchronized, but then they will synchronize on an instance of Subject and not on the collection instance.

However, in your case the problem is connected to what your update() does.
Are you sure your HexaObserver's update should create a new HexaObserver? Because you are adding a new instance of the same class to a collection which already contains that instance.

Dariusz
  • 21,561
  • 9
  • 74
  • 114
  • Yeah, it's because of the `new HexaObserver` I mention in my answer. You are iterating and modifying the same collection from the same thread. – Dariusz May 06 '15 at 11:37
0

ConcurrentModificationException is usually the sign that you used an Iterator on a Collection and that while iterating, you also modified the underlying Collection (remember that foreach-expression are actually a shortcut for using Iterator). The only way to solve this, is to iterate on a copy of the original collection. If you are in a multi-threaded environment, you also need to ensure that the copy of the collection is done in a thread-safe way.

So you could for example have:

public class Subject {
    private List<Observer> observers = new Vector<Observer>();
    private int state;
    public void setState(int state) {
          this.state = state;
          notifyAllObservers();
    }
    public void attach(Observer observer){
          observers.add(observer);      
    }
    public void notifyAllObservers(){
          for (Observer observer : ((List<Observer)observers.clone())) {
             observer.update();
           }
   }
   public void deattach(Observer observer) {
         observers.remove(observer);
   }
}
Guillaume Polet
  • 47,259
  • 4
  • 83
  • 117