Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Iterating a WeakHashMap

I'm using a WeakHashMap concurrently. I want to achieve fine-grained locking based on an Integer parameter; if thread A needs to modify a resource identified by Integer a and thread B does the same for resource identified by Integer b, then they need not to be synchronized. However, if there are two threads using the same resource, say thread C is also using a resource identified by Integer a, then of course thread A and C need to synchronize on the same Lock.

When there are no more threads that need the resource with ID X then the Lock in the Map for key=X can be removed. However, another thread can come in at that moment and try to use the lock in the Map for ID=X, so we need global synchronization when adding/removing the lock. (This would be the only place where every thread must synchronize, regardless of the Integer parameter) But, a thread cannot know when to remove the lock, because it doesn't know it is the last thread using the lock.

That's why I'm using a WeakHashMap: when the ID is no longer used, the key-value pair can be removed when the GC wants it.

To make sure I have a strong reference to the key of an already existing entry, and exactly that object reference that forms the key of the mapping, I need to iterate the keySet of the map:

synchronized (mrLocks){
    // ... do other stuff
    for (Integer entryKey : mrLocks.keySet()) {
        if (entryKey.equals(id)) {
            key = entryKey;
            break;
        }
    }
    // if key==null, no thread has a strong reference to the Integer
    // key, so no thread is doing work on resource with id, so we can
    // add a mapping (new Integer(id) => new ReentrantLock()) here as
    // we are in a synchronized block. We must keep a strong reference
    // to the newly created Integer, because otherwise the id-lock mapping
    // may already have been removed by the time we start using it, and 
    // then other threads will not use the same Lock object for this
    // resource
}

Now, can the content of the Map change while iterating it? I think not, because by calling mrLocks.keySet(), I created a strong reference to all keys for the scope of iteration. Is that correct?

like image 995
Timmos Avatar asked Jan 19 '15 11:01

Timmos


1 Answers

As the API makes no assertions about the keySet(), I would recommend a cache usage like this:

private static Map<Integer, Reference<Integer>> lockCache = Collections.synchronizedMap(new WeakHashMap<>());

public static Object getLock(Integer i)
{
    Integer monitor = null;
    synchronized(lockCache) {
        Reference<Integer> old = lockCache.get(i);
        if (old != null)
            monitor = old.get();

        // if no monitor exists yet
        if (monitor == null) {
            /* clone i for avoiding strong references 
               to the map's key besides the Object returend 
               by this method.
            */ 
            monitor = new Integer(i);
            lockCache.remove(monitor); //just to be sure
            lockCache.put(monitor, new WeakReference<>(monitor));
        }

    }

    return monitor;
}

This way you are holding a reference to the monitor (the key itself) while locking on it and allow the GC to finalize it when not using it anymore.

Edit:
After the discussion about payload in the comments I thought about a solution with two caches:

private static Map<Integer, Reference<ReentrantLock>> lockCache = new WeakHashMap<>();
private static Map<ReentrantLock, Integer> keyCache = new WeakHashMap<>();

public static ReentrantLock getLock(Integer i)
{
    ReentrantLock lock = null;
    synchronized(lockCache) {
        Reference<ReentrantLock> old = lockCache.get(i);
        if (old != null)
            lock = old.get();

        // if no lock exists or got cleared from keyCache already but not from lockCache yet
        if (lock == null || !keyCache.containsKey(lock)) {
            /* clone i for avoiding strong references 
               to the map's key besides the Object returend 
               by this method.
           */ 
            Integer cacheKey = new Integer(i); 
            lock = new ReentrantLock();
            lockCache.remove(cacheKey); // just to be sure
            lockCache.put(cacheKey, new WeakReference<>(lock));
            keyCache.put(lock, cacheKey);
        }                
    }

    return lock;
}

As long as a strong reference to the payload (the lock) exists, the strong reference to the mapped integer in keyCache avoids the removal of the payload from the lockCache cache.

like image 56
flo Avatar answered Oct 19 '22 09:10

flo