ConcurrentModificationException on ArrayList

Question

I'm getting an java.util.ConcurrentModificationException.

The related code is:

for (Iterator<Audit> it = this.pendingAudits.iterator(); it.hasNext();) {
    // Do something
    it.remove();
}

When it.remove() is called it throws me this exception.

This code is inside an @Serviceannotated class:

@Service
public class AuditService {
    public AuditService(
        this.pendingAudits = new ArrayList<Audit>();
    }

    public void flush(Audit... audits) {
        this.pendingAudits.addAll(Arrays.asList(audits));

        try {
            for (Iterator<Audit> it = this.pendingAudits.iterator(); it.hasNext();) {
                // Do something
                it.remove();
            }
        }
    }
}

The problem appears when two requests reach the code.

How could I avoid this concurrent access exception?

The problem appears when two requests reach the code - perhaps it's right time to think about synchronization? — rkosegi
– rkosegi, Commented Dec 4, 2018 at 14:08
@Lorelorelore iterators obtained from SynchronizedList must be manually synched by their users — Grzegorz Piwowarek
– Grzegorz Piwowarek, Commented Dec 4, 2018 at 14:21

Grzegorz Piwowarek · Accepted Answer · 2018-12-04 14:22:53Z

3

First things first, this is not a Spring-related problem. It's just a problem with a concurrent modification of one not-so-concurrent-friendly ArrayList class.

The simplest solution possible would be to synchronize access to the method that modifies it.

public synchronized void flush(Audit... audits) { }

Keep in mind that it will enforce the sequential execution of the flush method which imposes a huge performance penalty.

Sidenote, it's not enough to synchronize the collection itself by using Collections.synchronizedList - iterator instances returned by synchronized wrappers require manual synchronization.

edited Dec 4, 2018 at 14:22

answered Dec 4, 2018 at 14:17

Grzegorz Piwowarek

14.2k8 gold badges73 silver badges97 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

dharam · Accepted Answer · 2018-12-04 14:49:41Z

Isn't it obvious? You are sharing data without proper synchronization.

@Service annotated class generally is a singleton scope class and hence the same instance will be shared among all calling threads.

This results into all threads accessing the flush method on the same instance.

And guess what?

Your flush method is trying to modify an ArrayList which is a member variable. This makes it unsafe in a multithreaded scenario.

It's a good time to revisit the documentation of ArrayList which tells a lot more things about it's iterator.

Note that this implementation is not synchronized. If multiple threads access an ArrayList instance concurrently, and at least one of the threads modifies the list structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more elements, or explicitly resizes the backing array; merely setting the value of an element is not a structural modification.) This is typically accomplished by synchronizing on some object that naturally encapsulates the list. If no such object exists, the list should be "wrapped" using the Collections.synchronizedList method. This is best done at creation time, to prevent accidental unsynchronized access to the list: List list = Collections.synchronizedList(new ArrayList(...)); The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future. Note that the fail-fast behavior of an iterator cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast iterators throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: the fail-fast behavior of iterators should be used only to detect bugs.

Valeamor · Accepted Answer · 2018-12-05 02:20:06Z

0

I think you probably put Audit into the list pendingAudits somewhere and you want to flush them all when you call the flush(Audit...) method, you can use ConcurrentLinkedQueue instead of ArrayList.

public AuditService(
    this.pendingAudits = new ConcurrentLinkedQueue<Audit>();
}

public void flush(Audit... audits) {
    this.pendingAudits.addAll(Arrays.asList(audits));

    try {
        Audit audit;
        while ((audit = this.pendingAudits.poll) != null) {
            // Do something
        }
    }
}

edited Dec 5, 2018 at 2:20

answered Dec 5, 2018 at 1:51

Valeamor

531 silver badge6 bronze badges

Collectives™ on Stack Overflow

ConcurrentModificationException on ArrayList

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related