Uploaded image for project: 'OpenAM'
  1. OpenAM
  2. OPENAM-1660

Read-access to SubjectEvaluationCache is not synchronized

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Duplicate
    • Affects Version/s: Snapshot9.5.1, Snapshot9.5.2_RC1, Snapshot9.5.2, 9.5.3_RC1, 9.5.3, 9.5.4_RC1, 9.5.4, 10.0.0-EA, 10.0.0, 11.0.0, 11.0.1, 11.0.2, 11.0.3, 12.0.0, 12.0.1, 12.0.2, 12.0.3, 12.0.4, 13.0.0, 13.5.0, 13.5.1
    • Fix Version/s: None
    • Component/s: entitlements, policy
    • Support Ticket IDs:

      Description

      any access to shared state should be synchronized.

      From the Java SE API - doc

      "Note that this implementation is not synchronized. If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally. (A structural modification is any operation that adds or deletes one or more mappings; merely changing the value associated with a key that an instance already contains is not a structural modification.) This is typically accomplished by synchronizing on some object that naturally encapsulates the map. If no such object exists, the map should be "wrapped" using the Collections.synchronizedMap method. This is best done at creation time, to prevent accidental unsynchronized access to the map:"

      'read-access' to the caches is not synchronized ....

      SubjectEvaluationCache.java
      
          public static Boolean isMember(String tokenID, 
              String ldapServer, String valueDN ) 
          {
              Boolean member = null;
              String subjectId = ldapServer+":"+valueDN;
              Map subjectEntries = null;
              if ((subjectEntries = (Map)subjectEvaluationCache.get(tokenID))
                  != null) 
              {
                  long[] element = (long[])subjectEntries.get(subjectId);
                  long timeToLive = 0;
                  if (element != null) {
                      timeToLive = (long)element[0];
                      long currentTime = System.currentTimeMillis();
                      if (timeToLive > currentTime) {
                          if (DEBUG.messageEnabled()) {
                              DEBUG.message("SubjectEvaluationCache.isMember():"
                                + " get the membership result from cache.\n");
                          }
                          member = (element[1] == 1) ? Boolean.TRUE: 
                              Boolean.FALSE;
                      }
                  }
              }
              return member; 
          }
      
      
      However write-access is synchronized
      
          public static void addEntry(
              String tokenID, 
              String ldapServer,
              String valueDN, 
              boolean member
          ) {
              String subjectId = ldapServer+":"+valueDN;
              Map subjectEntries = null;
              long[] elem = new long[2];
              synchronized (subjectEvaluationCache) {
                  elem[0] = System.currentTimeMillis() + getSubjectEvalTTL();
                  elem[1] = (member == true) ? 1:0;
                  if ((subjectEntries = (Map)subjectEvaluationCache.get(tokenID))
                      != null) 
                  {
                      subjectEntries.put(subjectId, elem);
                  } else {
                      subjectEntries = Collections.synchronizedMap(new HashMap());
                      subjectEntries.put(subjectId, elem);
                      subjectEvaluationCache.put(tokenID,subjectEntries);
                  }
              }
          }
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                bthalmayr Bernhard Thalmayr
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: