1

I implemented a cache with using WeakHashMap and ReentrantReadWriteLock, my code is like that:

class Demo<T, K> {

    private final ReentrantReadWriteLock LOCK = new ReentrantReadWriteLock();

    private final Map<T, K> CACHE = new WeakHashMap<>();

    public K get(T t) {
        ReentrantReadWriteLock.ReadLock readLock = LOCK.readLock();
        ReentrantReadWriteLock.WriteLock writeLock = LOCK.writeLock();

        readLock.lock();
        if(CACHE.containsKey(t)){

            //-- question point --

            K result = CACHE.get(t);
            readLock.unlock();
            return result;
        }
        readLock.unlock();

        K result = // find from db;

        writeLock.lock();
        CACHE.put(t,result);
        writeLock.unlock();

        return result;
    }
}

My question is that whill it happen that gc is excuted after if(CACHE.containsKey(t)) but before K result = CACHE.get(t); with read lock and lead to that if(CACHE.containsKey(t)) is true but K result = CACHE.get(t); get null.

梁雨生
  • 385
  • 3
  • 16
  • As a side-node, the way you use locking is going tot give you problems in the future (e.g. unlocking twice, no safeguard efc.). In the javadoc of the lock class is an example of how you can safly use locks (by using a try-finally block). – n247s Apr 09 '19 at 04:26
  • Just don’t use this double-lookup. Just use `K result = CACHE.get(t); if(result != null) return result;`. Problem solved. Besides that, you are releasing the read lock, then acquiring the write lock; if another thread puts a value right between these two steps, you’ll overwrite the mapping. You have to *re-check* once acquired the write lock. Consider `putIfAbsent` (if you’re using Java 8). – Holger Apr 09 '19 at 08:39
  • @Holger Thanks for your suggestion very mush , it's very useful to me. – 梁雨生 Apr 09 '19 at 11:29

2 Answers2

2

Your ReentrantReadWriteLock has no control over the behavior of the WeakHashMap with regards to the garbage collector.

The class javadoc for WeakHashMap states

The behavior of the WeakHashMap class depends in part upon the actions of the garbage collector, so several familiar (though not required) Map invariants do not hold for this class. Because the garbage collector may discard keys at any time, a WeakHashMap may behave as though an unknown thread is silently removing entries. In particular, even if you synchronize on a WeakHashMap instance and invoke none of its mutator methods, it is possible for the size method to return smaller values over time, for the isEmpty method to return false and then true, for the containsKey method to return true and later false for a given key, for the get method to return a value for a given key but later return null, for the put method to return null and the remove method to return false for a key that previously appeared to be in the map, and for successive examinations of the key set, the value collection, and the entry set to yield successively smaller numbers of elements.

In other words, yes, it's possible for your containsKey call to return true and the get that follows to return false, if the garbage collector acts in between the two calls (and you have no other strong references to the corresponding key).

You can verify this behavior with a small program like

public class Example {
    public static void main(String[] args) throws Exception {
        WeakHashMap<Example, Integer> CACHE = new WeakHashMap<>();
        CACHE.put(new Example(), 2);
        if (CACHE.containsKey(new Example())) {
            System.gc();
            System.out.println("missing? " + CACHE.get(new Example()));
        }
    }

    @Override
    public int hashCode() {
        return 42;
    }

    @Override
    public boolean equals(Object obj) {
        return true;
    }
}

which prints

missing? null
Sotirios Delimanolis
  • 274,122
  • 60
  • 696
  • 724
1

Then your code will return null.

If that is not what you want, just do the get() call and check if you got a non-null result. There is no gain in calling containsKey() here like you do if you worry about returning null.

GhostCat
  • 137,827
  • 25
  • 176
  • 248