I have code which implements a "lock handler" for arbitrary keys. Given a key
, it ensures that only one thread at a time can process
that(or equals) key (which here means calling the externalSystem.process(key)
call).
So far, I have code like this:
public class MyHandler {
private final SomeWorkExecutor someWorkExecutor;
private final ConcurrentHashMap<Key, Lock> lockMap = new ConcurrentHashMap<>();
public void handle(Key key) {
// This can lead to OOM as it creates locks without removing them
Lock keyLock = lockMap.computeIfAbsent(
key, (k) -> new ReentrantLock()
);
keyLock.lock();
try {
someWorkExecutor.process(key);
} finally {
keyLock.unlock();
}
}
}
I understand that this code can lead to the OutOfMemoryError
because no one clear map.
I think about how to make map which will accumulate limited count of elements. When limit will be exceeded then we should replace oldest access element with new(this code should synchronized with oldest element as monitor). But I don't know how to have callback which will say me that limit exceeded.
Please share your thoughts.
P.S.
I reread the task and now I see that I have limitation that handle
method cannot be invoked more than 8 threads. I don't know how can it help me but I just mentioned it.
P.S.2
by @Boris the Spider was suggested nice and simple solution:
} finally {
lockMap.remove(key);
keyLock.unlock();
}
But after Boris noticed that code us not thread safe because it break behavior:
lets research 3 threads invoked with equally key:
- Thread#1 acquire the lock and now before
map.remove(key);
- Thread#2 invokes with equals key so it wait when thread#1 release lock.
- then thread#1 execute
map.remove(key);
. After this thread#3 invokes methodhandle
. It checks that lock for this key is absent in map thus it creates new lock and acquires it. - Thread#1 releases the lock and thus thread#2 acquires it.
Thus thread#2 and thread#3 can be invoked in parallel for equals keys. But it should not be allowed.
To avoid this situation, before map clearing we should block any thread to acquire the lock while all threads from waitset is not acquire and release the lock. Looks like it is enough complicated synchronization needed and it will lead to slow algorithm working. Maybe we should clear map from time to time when map size exceeds some limited value.
I wasted a lot of time but unfortunately I have not ideas how to achieve this.