Problem
If Im not mistaken - the Problem here is, that you have two synchronized blocks
- one for iterating
- and a second for removing elements.
Both blocks are locked only for itself. It is not the List what is locked like you might know it from DB-Recources and DB-Transactions.
So you can call two different synchronized blocks at the same time. If you do that on the same Object, you end up with to concurrent calls on the same Object.
for Example:
private List list
public void synchronized read() {
...iterate over list
}
public void synchronized remove() {
... remove some elements in list
}
And you have two Threads A and B
Then A and B cannot call read() at the same time and A and B cannot call remove at the same time. But they can call read() and remove() at the same time.
Solution
Try to use java.util.concurrent.locks.ReentrantReadWriteLock instead of the keyword synchronized. The Lock-Interface is new since Java 1.5 and is the modern way to do synchronization.
The following shows a class that own a buffer that will be read and changed by 3 Threads parallel:
class Container {
private List<String> buffer = Collections.synchronizedList(new ArrayList<String>());
private ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true);
private Lock writeLock = lock.writeLock();
private Lock readLock = lock.readLock();
public void readBuffer() {
readLock.lock();
Iterator<String> it = buffer.iterator();
while(it.hasNext()) {
it.next();
}
readLock.unlock();
}
public void addOne() {
writeLock.lock();
buffer.add("next");
writeLock.unlock();
}
public void removeOne() {
writeLock.lock();
if (buffer.size() > 0) {
buffer.remove(0);
}
writeLock.unlock();
}
}
For testing you can remove the readLock. That will lead to a ConcurrentModificationException. The following main() starts the test-Threads:
public class Concurrent {
public static Container container = new Container();
public static void main(String[] args) {
new Thread(new Filler()).start();
new Thread(new Killer()).start();
new Thread(new Reader()).start();
}
static class Filler implements Runnable {
@Override
public void run() {
while(true) {
container.addOne();
}
}
}
static class Killer implements Runnable {
@Override
public void run() {
while(true) {
container.removeOne();
}
}
}
static class Reader implements Runnable {
@Override
public void run() {
while(true) {
container.readBuffer();
}
}
}
}
The point is, that you use the same LockObject writeLock in two methods. Additionally this kind of Lock allows it to read the Data parallel. If it is necessary to allow only one Thread at a time to read the Data you could use ReentrantLock instead of ReentrantReadWriteLock