I have a collection. Many threads should be able to read from it at a time, but only one thread should be able to write to it at a time, and only when it's not being read. Java's ReentrantReadWriteLock seems prefect for this.
However, I am confused about how to write the iterator for the collection. The iterator should obtain the read lock when it starts. But I can't figure out how to ensure that it will unlock in the case when the iterator never finished.
Here's some example code that is just a wrapper around a normal iterator that locks and unlocks:
import java.util.Iterator;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
public class ReadLockIterator<T> implements Iterator<T> {
private final Lock lock;
private final Iterator<T> iterator;
public ReadLockIterator(ReadWriteLock lock, Iterator<T> iterator) {
this.lock = lock.readLock();
this.iterator = iterator;
this.lock.lock();
}
@Override
public boolean hasNext() {
return iterator.hasNext();
}
@Override
public T next() {
try {
return iterator.next();
}
finally {
if(!hasNext())
lock.unlock();
}
}
}
This will work fine as long as the user gets every element from the iterator. But what happens if the user doesn't do that? How can I ensure that the lock will eventually be released, even if some elements are never read from the iterator?
My first thought was to put a second check in the iterator's finalize() method, but then I read that finalize should not be used for unlocking.
What's the best way to handle this?
I'd copy the behavior of the standard java collections: iterator methods should throw ConcurrentModificationException if any write happened since the creation of the iterator.
That'd make your collection easy to understand for other developers.
Your collection could also provide a read lock which can be used to guarantee that no modification happen (and no ConcurrentModificationException is thrown) while the iterator is in use.
Another (probably easier to implement) option would be to allow to create and use iterators only when the current thread owns the read lock for the collection.
If an attempt to create an iterator is made when collection.getReadLock() isn't held by the current thread, then IllegalStateException (or some custom exception) should be thrown.
Similarly every method of the iterator should check that it's invoked in the thread it was created in, and that the read lock, that was held when this iterator was created, hasn't been released yet.
This way the code for the iterator can be mostly single-threaded, which usually significantly less complicated than multi-threading code.
In both cases the iterator can be used like that:
var collection = ...;
var readLock = collection.getReadLock();
readLock.lock();
try {
for (var item: collection) {
process(item);
}
} finally {
readLock.unlock();
}
Making a lock Iterable (as suggested in this answer) makes the code non-intuitive in my opinion.
I'd suggest renaming "lock" into "snapshot" or something along these lines in that case.