Concurrent Modification Exception in LinkedList

Question

I'm looking for a good way, to build a limited linked list. If the linked list is "full", the first element will be removed and the new one will be added. So I always have the "newest" "limit-size" elements.

This is implemented in the following way:

    private int maxSize;

public LimitedLinkedList(int maxSize) {
    this.maxSize = maxSize;
}

@Override
public synchronized boolean add(E object) {
    boolean success = super.add(object);
    while (this.size() >= maxSize) {
        removeFirst();
    }
    return success;
}

Now I have the following problem: I need to calculate the average of the linked-list. This is the moment, where I randomly get the concurrent modification exception or an index-out-of-bounds exception. My method for average:

public synchronized static double movingAverage(
        LinkedList<AverageObject> valueList) {
    if (valueList.isEmpty()) {
        return 0;
    }
    double sum = 0;

    int m = 0;
    for (int i = 0; i < valueList.size(); i++) {
        AverageObject object= valueList.get(i);
        sum += object.value;
        m++;
    }

    sum = (m != 0) ? sum / m : sum;
    return sum;
 }

Do you know a good way to avoid concurrent modification exceptions?

My only idea is, to calculate the average, everytime the list is changed, so I don't have to iterate through it, when I want to have the average.

Stephen C · Accepted Answer · 2013-05-14 22:36:28Z

The concurrent modification problem is actually nothing to do with your modifications to add. It would occur with a regular LinkedList too ... if you added an element while computing the average. It is also worth nothing that the code you showed cannot generate a ConcurrentModificationException at all. (But it could give out-of-bounds exceptions ...)

The most likely reason you are having problems here is that your add and movingAverage methods are not synchronizing correctly:

A synchronized instance method locks the target object; i.e. the list instance.
a static synchronized method locks the Class object for the method's declaring class; i.e. the class that declares your movingAverage method.

If two threads don't lock the same object, they won't synchronize, and you won't get mutual exclusion. This means that the add and movingAverage could be simultaneously reading and updating the same list ... resulting in an exception (or worse).

One way to avoid these problems might be to change the movingAverage method to this:

public static double movingAverage(
    LinkedList<AverageObject> valueList) {
    synchronized (valueList) {
       ...
    }
}

or even this:

public synchronized doubkle movingAverage() {
    ...
}

However, this is all piece-meal. A better approach might be to synchronize at a higher level, or use a "concurrent" data structure that avoids the need for explicit synchronizing.

howiewylie · Accepted Answer · 2013-05-14 11:21:54Z

2

In your code example you synchronized the movingAverage method which means that access to the static method is thread safe. However, your list as the parameter passed to it is not. It can still be modified at the same time as you check for the averages by another object that calls the add method of the list. If your synchronized movingAverage() method would live within the LimitedLinkedList object then the operation would be thread safe in regards to the add method.

answered May 14, 2013 at 11:21

howiewylie

1714 bronze badges

Comments

Satya · Accepted Answer · 2013-05-14 11:26:36Z

1

Try something like this ( only for optimizing your code, It may also solve your concurrent modification exception):

public class LimitedLinkedList extends LinkedList<AverageObject>{
   private int maxSize;
       private int sum = 0;


public LimitedLinkedList(int maxSize) {
    this.maxSize = maxSize;
}

@Override
public synchronized boolean add(AverageObject object) {
    sum = sum + object.value;
    boolean success = super.add(object);
    while (this.size() >= maxSize) {
        sum = sum - getFirst().value; 
        removeFirst();

    }
    return success;
}


public synchronized double movingAverage(int sum, int length) {

    double avegage = (sum != 0) ? sum / length : sum;
    return sum;
 }

}

answered May 14, 2013 at 11:26

Satya

8,38610 gold badges42 silver badges44 bronze badges

Comments

John B · Accepted Answer · 2013-05-14 11:44:10Z

1

Use a synchronizedList via Collections.synchronizedList and follow the javadoc instructions for iteration.

FYI, there is a ConcurrentRunningAverage checked into GitHub that you could use or use as a guide. It extends BasicRunningAverge.

edited May 14, 2013 at 11:44

answered May 14, 2013 at 11:00

John B

33.1k8 gold badges80 silver badges100 bronze badges

Collectives™ on Stack Overflow

Concurrent Modification Exception in LinkedList

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related