Correct way to synchronize ArrayList in java
Solution 1
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.
Solution 2
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.
Solution 3
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().
Solution 4
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
bob
Updated on January 07, 2020Comments
-
bob over 4 years
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 theregisterInQueue
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); } }
-
Bombe over 14 yearsSynchronizing 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 14 yearsso I should do something like: synchronized ((List)in_queue) ?
-
bob over 14 yearsOk! I actually removed a bit of code to make it simple. I won't have a problem with clear()/remove(). thanks =]
-
bob over 14 yearsThanks 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 14 yearsNote there's a LinkedBlockingQueue as well. And you don't necessarily need to impose limits.
-
Michael Borgwardt over 14 years@Bombe: no, it is completely pointless. As I wrote, the use of Collections.synchronizedList() is superfluous
-
bob over 14 yearsWhen I try manual synchronization I get an error message "field not static final" when I use synchronized (in_queue). Should this be replaced with synchronized ((List)in_queue) ?
-
Michael Borgwardt over 14 yearsNo, 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.