Synchronized ArrayList editing in multiple threads

Question

I have multiple threads of the same kind that are sharing the ArrayList:

static ArrayList<Integer> Terminate_List = new ArrayList<Integer>();

This ArrayList needs to be updated. I need to delete a single item of that list. The problem is when I reach the line

Terminate_List.remove(k);

The other thread already deleted another Item of the list, the size has decremented and I get a IndexOutOfBoundsException. So my plan was to make this method synchronized so that only one thread at a time can execute this method. The method is executed by a timer. How can I really let only one thread at a time execute this method?

    public synchronized void update_list(){

   for (int k = 0; k < Terminate_List.size(); k++) {

        if (Terminate_List.get(k) == this_ID){

            Terminate_List.remove(k); }}}

Use a static variable and put the update_list() in a loop if the static variable boolean is true then it is checked out if false then make the update. — brso05
– brso05, Commented Oct 15, 2014 at 20:25
You should synchronize all updates to the list against the same object. We can't tell if this is the case because we don't know where update_list() is. — biziclop
– biziclop, Commented Oct 15, 2014 at 20:25
@brso05 I'm sorry but that's a horrible idea. It is wrong but its failure won't be immediately apparent because it will work most of the time. — biziclop
– biziclop, Commented Oct 15, 2014 at 20:26
@Hydrophilia The probable reason is that you have several instances of whichever class update_list() is in. Without seeing more code it's impossible to tell for sure. — biziclop
– biziclop, Commented Oct 15, 2014 at 20:27
Or even better: use a proper collection that supports concurrency. For List, you have CopyOnWriteArrayList. But IMO it would be better using another approach for your problem like a ConcurrentHashMap instead. — Luiggi Mendoza
– Luiggi Mendoza, Commented Oct 15, 2014 at 20:30

Krease · Accepted Answer · 2014-10-15 20:45:51Z

1

Don't use remove by index - use remove by reference. This way you don't have to worry about indexes at all.

Also, Terminate_List is static, while your method is not. That means if you have multiple instances of your class, blocking will only occur where two threads area accessing the same instance. To block other threads from updating Terminate_List, you need to update how you're using synchronized.

Your code would become this:

public void update_list(){
   // your previous impl was equivalent to synchronized(this)
   synchronized (Terminate_List) {  
       Terminate_List.remove((Integer)this_ID);
   }
}

Note - I'm assuming this_ID is defined as an int - if it's already an Integer then you don't need the cast (and you'd have a problem with your equality check in your original code).

edited Oct 15, 2014 at 20:45

answered Oct 15, 2014 at 20:30

Krease

16.3k8 gold badges58 silver badges92 bronze badges

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

2 Comments

biziclop Over a year ago

Since the list contains Integers, and this_ID is probably an int, an explicit cast would be needed here.

Krease Over a year ago

There's no mention in the question on what this_ID is; though based on the OP using == for comparison instead of equals, you're probably right. I'll update the answer.

kapex · Accepted Answer · 2014-10-15 20:31:17Z

0

This probably happens because you have multiple instances of whatever class update_list() is in and a static list that is used by them all. If this true, then you need to synchronize using the list as monitor:

public void update_list() {
  synchronized (terminateList) {
      // ...
  }
}

answered Oct 15, 2014 at 20:31

kapex

30.2k6 gold badges117 silver badges128 bronze badges

Collectives™ on Stack Overflow

Synchronized ArrayList editing in multiple threads

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related