i get ConcurrentModificationException when adding an item [duplicate]

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
– thatidiotguy, Commented Jun 4, 2014 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
– thatidiotguy, Commented Jun 4, 2014 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
– dimoniy, Commented Jun 4, 2014 at 17:41

Syam S · Accepted Answer · 2014-06-05 09:57:07Z

0

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

edited Jun 5, 2014 at 9:57

answered Jun 4, 2014 at 18:18

Syam S

8,5071 gold badge29 silver badges37 bronze badges

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

6 Comments

abdel elk Over a year ago

thank you a lot but i tried all those solution but doesnt work

Syam S Over a year ago

Oops. Thats weird. Ideally you should not get ConcurrentModificationException with CopyOnWriteArrayList. Can you post some exception trace?

abdel elk Over a year ago

i changed it to set you can look i posted the complete function

yshavit Over a year ago

@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 stackoverflow.com/questions/1655362.

Syam S Over a year ago

Answer modified based on you question update. Hope this helps.

|

Collectives™ on Stack Overflow

i get ConcurrentModificationException when adding an item [duplicate]

1 Answer 1

6 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Linked

Related