EHC ❯ DiskStore.evictLfuDiskElements() -> synchronized (diskElements) not necessary

• Type: Bug
• Status: Closed
• Priority: 2 Major
• Resolution: Fixed
• Component/s:
• Labels:

• Assignee: hsingh
• Reporter: mads1980
• Created: May 24, 2010
• Votes: 0
• Watchers: 2
• Updated:

  January 17, 2013
• Resolved:

  May 25, 2010

Attachments

• mads1980 (53.00 k) text/plain DiskStore.java

Description

Please see the following code at DiskStore.evictLfuDiskElements():

private void evictLfuDiskElements(int count) {
    synchronized (diskElements) {
        for (int i = 0; i < count; i++) {
            DiskElement diskElement = findRelativelyUnused();
            diskElements.remove(diskElement.key);
            notifyEvictionListeners(diskElement);
            freeBlock(diskElement);
        }
    }
}

The field diskElements is a ConcurrentHashMap, so synchronization on the instance would not be necessary, at least not for maintaining Map consistency. Even if it were necessary for any other reasons, this is the only block of code synchronizing on this mutex. As far as I can see, the only possible stack trace would be:

-> flushSpool() -> writeOrReplaceEntry() -> evictLfuDiskElements()

flushSpool() is already synchronized on the DiskStore’s implicit instance mutex, so there’s no possibility of concurrently reaching the synchronized (diskElements) block.

Comments

Manuel Dominguez Sarmiento 2010-05-25

DiskStore patch including mods for EHC-721, EHC-722, EHC-723, EHC-725, and EHC-726

Himadri Singh 2010-06-27

Tweaked patch has been checked in.