In a task they asked me to create a tread safe custom HashMap.
I got an idea from Collections.SynchronizedCollection of java and created this class.
public final class SynchronizedHashMap<K, V> extends HashMap<K, V> {
private final Object lock = new Object();
public SynchronizedHashMap(int initialCapacity, float loadFactor) {
super(initialCapacity, loadFactor);
}
public SynchronizedHashMap(int initialCapacity) {
super(initialCapacity);
}
public SynchronizedHashMap() {
super();
}
public SynchronizedHashMap(Map<? extends K, ? extends V> m) {
super(m);
}
private void sync(Runnable runnable) {
synchronized (lock) {
runnable.run();
}
}
private <T> void sync(Consumer<T> consumer, T t) {
synchronized (lock) {
consumer.accept(t);
}
}
private <T> T sync(Supplier<T> supplier) {
synchronized (lock) {
return supplier.get();
}
}
private <T, R> R sync(Function<T, R> function, T t) {
synchronized (lock) {
return function.apply(t);
}
}
private <T, U, R> R sync(BiFunction<T, U, R> function, T t, U u) {
synchronized (lock) {
return function.apply(t, u);
}
}
@Override
public int size() {
return sync(super::size);
}
@Override
public boolean isEmpty() {
return sync(super::isEmpty);
}
@Override
public V get(Object key) {
return sync(super::get, key);
}
@Override
public boolean containsKey(Object key) {
return sync(super::containsKey, key);
}
@Override
public V put(K key, V value) {
return sync(super::put, key, value);
}
@Override
public void putAll(Map<? extends K, ? extends V> m) {
sync(super::putAll, m);
}
@Override
public V remove(Object key) {
synchronized (lock) {
return super.remove(key);
}
}
@Override
public void clear() {
sync(super::clear);
}
@Override
public boolean containsValue(Object value) {
return sync(super::containsValue, value);
}
@Override
public Set<K> keySet() {
return sync(super::keySet);
}
@Override
public Collection<V> values() {
return sync(super::values);
}
@Override
public Set<Entry<K, V>> entrySet() {
return sync(super::entrySet);
}
@Override
public V getOrDefault(Object key, V defaultValue) {
return sync(super::getOrDefault, key, defaultValue);
}
@Override
public V putIfAbsent(K key, V value) {
return sync(super::putIfAbsent, key, value);
}
@Override
public boolean remove(Object key, Object value) {
return sync(super::remove, key, value);
}
@Override
public boolean replace(K key, V oldValue, V newValue) {
synchronized (lock) {
return super.replace(key, oldValue, newValue);
}
}
@Override
public V replace(K key, V value) {
return sync(super::replace, key, value);
}
@Override
public V computeIfAbsent(K key, Function<? super K, ? extends V> mappingFunction) {
return sync(super::computeIfAbsent, key, mappingFunction);
}
@Override
public V computeIfPresent(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
return sync(super::computeIfPresent, key, remappingFunction);
}
@Override
public V compute(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
return sync(super::compute, key, remappingFunction);
}
@Override
public V merge(K key, V value, BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
synchronized (lock) {
return super.merge(key, value, remappingFunction);
}
}
@Override
public void forEach(BiConsumer<? super K, ? super V> action) {
sync(super::forEach, action);
}
@Override
public void replaceAll(BiFunction<? super K, ? super V, ? extends V> function) {
sync(super::replaceAll, function);
}
@Override
public Object clone() {
return sync(super::clone);
}
@Override
public boolean equals(Object o) {
return sync(super::equals, o);
}
@Override
public int hashCode() {
return sync(super::hashCode);
}
@Override
public String toString() {
return sync(super::toString);
}
}
My questions are:
is this a thread safe class ?
Can I say this is a wrapper around HashMap class?
actually I don't know how to write unit test for testing thread safe too.
Update
I added and changed this methods from Stephen's answer.
@Override
public Set<K> keySet() {
return sync(this::synchronizedKeySet);
}
private Set<K> synchronizedKeySet() {
return Collections.synchronizedSet(super.keySet());
}
@Override
public Collection<V> values() {
return sync(this::synchronizedValues);
}
private Collection<V> synchronizedValues() {
return Collections.synchronizedCollection(super.values());
}
@Override
public Set<Entry<K, V>> entrySet() {
return sync(this::synchronizedEntrySet);
}
private Set<Entry<K, V>> synchronizedEntrySet() {
return Collections.synchronizedSet(super.entrySet());
}
Can you please give feedback too?
But since the lock in Collections synchronized methods is deferent from min am I have to make other synchronized set and collection class too?
CodePudding user response:
is this a thread safe class ?
As far as I can tell, mostly yes.
However, the keySet
, values
and entrySet
methods all return collections implemented by HashMap
. They are not synchronized in any way, so they are not thread-safe. And since they can be used to access and update the SynchronizedHashMap
itself, this will compromise the thread-safety of the other methods.
Note that java.utils.Collections
handles this by having those methods return synchronized collections that share the same lock object as the parent Map
.
But even this has a caveat on thread-safety; see the javadoc for Collections.synchronizeMap
for details.
The updated version is still not correct. The collections returned by those 3 methods need to lock the same mutex as you are using in your SynchronizedHashMap
class. There is no easy way to do that.
Can I say this is a wrapper around
HashMap
class?
Technically, no. There is no distinct HashMap
inside the "wrapper".
I don't know how to write unit test for testing thread safe too.
Testing thread-safety is difficult. Testing can only demonstrate that a class is NOT thread-safe. It can't show that it is.
You are better of analyzing the code. Or better still, use the concurrency classes provided by the Java SE libraries ... which have been thoroughly tested AND analyzed by programmers who really understand concurrency. (And reviewed by thousands of other people who like reading OpenJDK source code.)