i get ConcurrentModificationException when adding an item

Question

im trying to save the manager id of distinct managers from collabs to managersId but i get an exeption "ConcurrentModificationException"

public void fillTree() throws SystemException, PortalException {

        TreeNode nodeParent;
        TreeNode nodeFils;
        Set<Long> managersId = new HashSet<Long>();
        UserVO user = new UserVO();
        collabs = CollabLocalServiceUtil.getCollabs(-1, -1);
        Iterator<Long> iter = managersId.iterator();
        long id;
        for (int i = 0; i < collabs.size(); i++) {
            id = collabs.get(i).getManagerId();
            synchronized (managersId) {
                managersId.add((Long) id);
                System.out.println(id);

            }

        }



        while (iter.hasNext()) {
            id = iter.next();//throw exeption
            user = getUserById(id);
            nodeParent = new DefaultTreeNode(user.getFullName(), root);
            for (int j = 0; j < collabs.size(); j++) {
                if (collabs.get(j).getManagerId() == user.getUserId()) {

                    nodeFils = new DefaultTreeNode(getUserById(
                            collabs.get(j).getUserId()).getFullName(),
                            nodeParent);
                }
            }
        }

    }

im using liferay portal

`ArrayList` is not synchronized so you need to handle multiple threads manipulating the data at once. — thatidiotguy, Jun 04 '14 at 17:33
That is an extremely complicated topic. There are whole books written about it. At its most basic, you need to create read and write locks so that threads are blocked from accessing the data when other threads are already accessing it. Or you could just use `Vector` instead of ArrayList as it is internally syncrhonized — thatidiotguy, Jun 04 '14 at 17:41
Is managersId a local variable? Do other threads have access to it? Do you open an iterator on it or use for-each syntax with it? — dimoniy, Jun 04 '14 at 17:41
If its local then try synchronized block inside for loop. Like `synchronized (managersId) { // if condition }` — Syam S, Jun 04 '14 at 18:05
From the code provided, **this has nothing to do with multithreading**. `ConcurrentModificationException` [can easily happen in a single-threaded program](http://stackoverflow.com/questions/1655362). But the code as written would also not cause a `ConcurrentModificationException`. OP, can you come up with a short, _complete_ (self-contained) example that shows your error? Something like this, which I adapted from your code (but which runs fine): https://gist.github.com/yshavit/c48362f9e40bc8770a0e — yshavit, Jun 04 '14 at 19:17
i don't think this code would throw ConcurrentModificationException. can please check once again — Gautam, Jun 04 '14 at 19:22

Syam S · Accepted Answer · 2014-06-05T09:57:07.147

Edit based on question update.

In this new code the issue is with your iterator. You initialized the iterator and then modified the collection and then trying to use the dirty iterator. That is the cause of concurrent modification exception. So the fix for this issue is simple. Just move Iterator<Long> iter = managersId.iterator(); after for loop. Try

public void fillTree() throws SystemException, PortalException {

    TreeNode nodeParent;
    TreeNode nodeFils;
    Set<Long> managersId = new HashSet<Long>();
    UserVO user = new UserVO();
    collabs = CollabLocalServiceUtil.getCollabs(-1, -1);        
    long id;
    for (int i = 0; i < collabs.size(); i++) {
        id = collabs.get(i).getManagerId();
        synchronized (managersId) {
        managersId.add((Long) id);
        System.out.println(id);

        }

    }

    Iterator<Long> iter = managersId.iterator(); // Getting the new iterator with latest value.
    while (iter.hasNext()) {
        id = iter.next();//Now this wont throw exeption
        user = getUserById(id);
        nodeParent = new DefaultTreeNode(user.getFullName(), root);
        for (int j = 0; j < collabs.size(); j++) {
        if (collabs.get(j).getManagerId() == user.getUserId()) {

            nodeFils = new DefaultTreeNode(getUserById(
                collabs.get(j).getUserId()).getFullName(),
                nodeParent);
        }
        }
    }
}

OLD ANSWER

Firstly from your logic I think you are trying to get unique manager id as list. In this case you could use a Set.

As for your current problem if its executing in a multi thread environment you could use synchronized block like

List<Long> managersId = new ArrayList<Long>();

collabs = CollabLocalServiceUtil.getCollabs(-1, -1);

long id;
for (int i = 0; i < collabs.size(); i++) {
    id = collabs.get(i).getManagerId();
    synchronized (managersId) {
        if (!managersId.contains(id)) {
            managersId.add((Long) id);
        }
    }
}

Or else you could use java.util.concurrent.CopyOnWriteArrayList for concurrent list operation.

List<Long> managersId = new CopyOnWriteArrayList<Long>();

Also as a third option you can make a normal list synchronized by collections class

Collections.synchronizedList(managersId);

thank you a lot but i tried all those solution but doesnt work — abdel elk, Jun 04 '14 at 18:47
Oops. Thats weird. Ideally you should not get ConcurrentModificationException with CopyOnWriteArrayList. Can you post some exception trace? — Syam S, Jun 04 '14 at 18:50
i changed it to set you can look i posted the complete function — abdel elk, Jun 04 '14 at 19:27
@abdelelk You definitely don't need `synchronized` for a thread-local variable; no other thread has access to it, so locking on it isn't necessary and doesn't do anything. This problem has absolutely nothing to do with multithreading. Your problem is that you first get the iterator, then modify the underlying collection, then try to use the iterator. That's not allowed. See http://stackoverflow.com/questions/1655362. — yshavit, Jun 04 '14 at 20:17
Answer modified based on you question update. Hope this helps. — Syam S, Jun 05 '14 at 09:57
thank you my error was that i call the method from another(getter) whos called duriin multiple phase and that generate me an exception hope that can help someone — abdel elk, Jun 06 '14 at 18:01

i get ConcurrentModificationException when adding an item

1 Answers1