Correct way to synchronize ArrayList in java

Question

I'm not sure if this is the correct way to synchronize my ArrayList.

I have an ArrayList in_queue which is passed in from the registerInQueue function.

ArrayList<Record> in_queue = null;

public void registerInQueue(ArrayList in_queue)
{
    this.in_queue = in_queue;
}

Now I'm trying to synchronize it. Is this sychronizing my in_queue object correctly?

List<Record> in_queue_list = Collections.synchronizedList(in_queue);

synchronized (in_queue_list) {
    while (in_queue_list.size() > 0) {
        in_queue_list.remove(0);
    }
}

You can't really upgrade a list to be synchronized because you are being passed a reference, so you don't really own it. Whatever gave you the reference could still modify the original list it passed you concurrently. Regardless of what synchronization you add, unless it is explicitly known that the list will be guarded by it's intrinsic lock. — Tim Bender
– Tim Bender, Commented Sep 16, 2009 at 8:46
I will be putting a synchronized block around any operation on the queue. Thanks! — bob
– bob, Commented Sep 16, 2009 at 9:09

Michael Borgwardt · Accepted Answer · 2009-09-16 11:31:07Z

50

You're synchronizing twice, which is pointless and possibly slows down the code: changes while iterating over the list need a synchronnization over the entire operation, which you are doing with synchronized (in_queue_list) Using Collections.synchronizedList() is superfluous in that case (it creates a wrapper that synchronizes individual operations).

However, since you are emptying the list completely, the iterated removal of the first element is the worst possible way to do it, sice for each element all following elements have to be copied, making this an O(n^2) operation - horribly slow for larger lists.

Instead, simply call clear() - no iteration needed.

Edit: If you need the single-method synchronization of Collections.synchronizedList() later on, then this is the correct way:

List<Record> in_queue_list = Collections.synchronizedList(in_queue);
in_queue_list.clear(); // synchronized implicitly,

But in many cases, the single-method synchronization is insufficient (e.g. for all iteration, or when you get a value, do computations based on it, and replace it with the result). In that case, you have to use manual synchronization anyway, so Collections.synchronizedList() is just useless additional overhead.

edited Sep 16, 2009 at 11:31

answered Sep 16, 2009 at 8:36

Michael Borgwardt

347k81 gold badges491 silver badges726 bronze badges

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

6 Comments

Bombe Over a year ago

Synchronizing twice here is not pointless: it ensures that while the loop is running nobody else can modify the list. Not using clear() is a bit over-the-top, though. :)

bob Over a year ago

so I should do something like: synchronized ((List)in_queue) ?

bob Over a year ago

Ok! I actually removed a bit of code to make it simple. I won't have a problem with clear()/remove(). thanks =]

Michael Borgwardt Over a year ago

@Bombe: no, it is completely pointless. As I wrote, the use of Collections.synchronizedList() is superfluous

Michael Borgwardt Over a year ago

No, that won't achieve anything. What is the exact error message and at what line does it occur? The error message "field not static final" can't be found via Google, so I doubt that's what you actually get.

|

Brian Agnew · Accepted Answer · 2009-09-16 08:49:00Z

8

Looking at your example, I think ArrayBlockingQueue (or its siblings) may be of use. They look after the synchronisation for you, so threads can write to the queue or peek/take without additional synchronisation work on your part.

answered Sep 16, 2009 at 8:49

Brian Agnew

273k38 gold badges342 silver badges443 bronze badges

2 Comments

bob Over a year ago

Thanks for the suggestion! That is what I am trying to do, not sure about limiting the size of my array though. I'll keep this in mind. ;)

Brian Agnew Over a year ago

Note there's a LinkedBlockingQueue as well. And you don't necessarily need to impose limits.

Andrew Duffy · Accepted Answer · 2009-09-16 08:33:05Z

6

That's correct, and documented:

http://java.sun.com/javase/6/docs/api/java/util/Collections.html#synchronizedList(java.util.List)

However, to clear the list, just call List.clear().

answered Sep 16, 2009 at 8:33

Andrew Duffy

6,9682 gold badges27 silver badges17 bronze badges

Comments

David Rabinowitz · Accepted Answer · 2009-09-16 08:33:53Z

5

Yes it is the correct way, but the synchronised block is required if you want all the removals together to be safe - unless the queue is empty no removals allowed. My guess is that you just want safe queue and dequeue operations, so you can remove the synchronised block.

However, there are far advanced concurrent queues in Java such as ConcurrentLinkedQueue

answered Sep 16, 2009 at 8:33

David Rabinowitz

30.5k16 gold badges95 silver badges125 bronze badges

Comments

Dhaval Patel · Accepted Answer · 2015-11-24 13:14:57Z

Let's take a normal list (implemented by the ArrayList class) and make it synchronized. This is shown in the SynchronizedListExample class. We pass the Collections.synchronizedList method a new ArrayList of Strings. The method returns a synchronized List of Strings. //Here is SynchronizedArrayList class

package com.mnas.technology.automation.utility;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import org.apache.log4j.Logger;
/**
* 
* @author manoj.kumar
* @email [email protected]
* 
*/
public class SynchronizedArrayList {
    static Logger log = Logger.getLogger(SynchronizedArrayList.class.getName());
    public static void main(String[] args) {    
        List<String> synchronizedList = Collections.synchronizedList(new ArrayList<String>());
        synchronizedList.add("Aditya");
        synchronizedList.add("Siddharth");
        synchronizedList.add("Manoj");
        // when iterating over a synchronized list, we need to synchronize access to the synchronized list
        synchronized (synchronizedList) {
            Iterator<String> iterator = synchronizedList.iterator();
            while (iterator.hasNext()) {
                log.info("Synchronized Array List Items: " + iterator.next());
            }
        }    
    }
}

Notice that when iterating over the list, this access is still done using a synchronized block that locks on the synchronizedList object. In general, iterating over a synchronized collection should be done in a synchronized block

Collectives™ on Stack Overflow

Correct way to synchronize ArrayList in java

5 Answers 5

6 Comments

2 Comments

Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

6 Comments

2 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related