CODEWITHSUNDEEP
javaconcurrencylinked-listsummary
Frame91
Frame91

Reputation: 3789

Concurrent Modification Exception in LinkedList

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.

Upvotes: 0

Views: 2437

Answers (4)

Stephen C
Stephen C

Reputation: 719239

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.

Upvotes: 5

John B
John B

Reputation: 32969

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.

Upvotes: 1

Satya
Satya

Reputation: 8346

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;
 }

}

Upvotes: 1

howiewylie
howiewylie

Reputation: 171

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.

Upvotes: 2

Related Questions