javaarrayliststaticjvmgarbage-collection

java static list causes memory leak


I have following code which seems to be causing some memory leaks. This code snippet(i.e. hasPermissions()) get executed every time when a user performs an action.

So from what I understood, since Map permissions is a static object, all permissionsList objects that are created inside hasPermission() method have references to global static object(i.e. permissions); therefore, is not eligible to be garbage collected?

Following is the Leak Suspect showing for its heap dump in Eclipse Memory Analyzer tool. When I navigate to details, it shows the class with following code snippet. I found Java List.addAll() function internally creates LinkedList. I'm still trying to comprehend what's exactly happening here. Appreciate your thoughts.

enter image description here

public class AccessManager {
    
    
        private static Map <Integer, List> permissions;
    
        public static void init()
        {
            //initiate permissions and add values
        }
        
        public static boolean hasPermissions(List<Integer> accessLevels, String action)
        {
    
            if (permissions == null)
                init();
            
            List permissionsList = null;
            
            for (Integer a : accessLevels) {
                
                if (permissionsList == null) {
                    permissionsList = permissions.get(a);
                } else {
                    permissionsList.addAll(permissions.get(a));
                }
            }
            
            if(permissionsList.contains(action)){
                return true;
            }
            
            return false;
        }
    
    }

Solution

  • That code is quite severely broken. In adding new permissions to a List referenced by permissions, the hasPermission method not only creates a memory leak, but may cause subsequent access checks to be incorrectly granted, which might be a quite serious security vulnerability.

    For instance, if permissions contains

    Key Value
    1 [a]
    2 [b]

    and you execute hasPermissions(Arrays.asList(1,2), "b"), permissions will be updated to contain

    Key Value
    1 [a, b]
    2 [b]

    causing a subsequent call to hasPermissions(Arrays.asList(1), "b") to return true.

    Also each further call to hasPermissions(Arrays.asList(1,2), "b") causes b to be added again:

    Key Value
    1 [a, b, b]
    2 [b]

    causing that list to get longer and longer until memory is exhausted. This extremely long list full of duplicates will slow down access checks such as hasPermissions(Arrays.asList(1), "c") to a crawl.

    Oh, and by the way, the lazy initialization that calls init on first invocation of hasPermissions is not thread safe, but potentially accessed by multiple threads. You should either declare permissions volatile (or even synchronize init if init is not safe for use by multiple threads), or invoke init in a static initializer.