Create custom Thread Safe HashMap

Question

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());
}

Is this change solves keySet, values and entrySet methods?

Answer 1

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.)