[java-opensaml COMMIT] /trunk/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynam...

[noreply at shibboleth.net]

Author: putmanb
Date: Tue Nov 18 17:42:53 2014
New Revision: 4153

URL: http://svn.shibboleth.net/view/java-opensaml?rev=4153&view=rev
Log:
Work rework eval of refresh trigger time to account for race condition.
This should avoid unnecessary duplicate fetches from concurrent threads that fell into resolveFromOriginSource b/c were past refresh trigger time. 

Modified:
    trunk/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java

Modified: trunk/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java
URL: http://svn.shibboleth.net/view/java-opensaml/trunk/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java?rev=4153&r1=4152&r2=4153&view=diff
==============================================================================
--- trunk/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java (original)
+++ trunk/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java Tue Nov 18 17:42:53 2014
@@ -290,13 +290,10 @@
         
         EntityManagementData mgmtData = getBackingStore().getManagementData(entityID);
         Lock readLock = mgmtData.getReadWriteLock().readLock();
-        boolean shouldAttemptRefresh = false;
         try {
             readLock.lock();
             
-            shouldAttemptRefresh = shouldAttemptRefresh(mgmtData);
-            
-            if (!shouldAttemptRefresh) {
+            if (!shouldAttemptRefresh(mgmtData)) {
                 List<EntityDescriptor> descriptors = lookupEntityID(entityID);
                 if (!descriptors.isEmpty()) {
                     log.debug("Found requested metadata in backing store, returning");
@@ -312,7 +309,7 @@
             readLock.unlock();
         }
         
-        return resolveFromOriginSource(criteria, shouldAttemptRefresh);
+        return resolveFromOriginSource(criteria);
     }
     
     /**
@@ -320,21 +317,23 @@
      * and then return it.
      * 
      * @param criteria the input criteria set
-     * @param resolveUnconditionally whether metadata should unconditionally be resolved, 
-     *           even if there is existing metadata
      * @return the resolved metadata
      * @throws ResolverException  if there is a fatal error attempting to resolve the metadata
      */
     @Nonnull @NonnullElements protected Iterable<EntityDescriptor> resolveFromOriginSource(
-            @Nonnull final CriteriaSet criteria, boolean resolveUnconditionally) throws ResolverException {
+            @Nonnull final CriteriaSet criteria) throws ResolverException {
         
         String entityID = StringSupport.trimOrNull(criteria.get(EntityIdCriterion.class).getEntityId());
-        Lock writeLock = getBackingStore().getManagementData(entityID).getReadWriteLock().writeLock(); 
+        EntityManagementData mgmtData = getBackingStore().getManagementData(entityID);
+        Lock writeLock = mgmtData.getReadWriteLock().writeLock(); 
         
         try {
             writeLock.lock();
             
-            if (!resolveUnconditionally) {
+            // It's possible that multiple threads fall into here and attempt to preemptively refresh. 
+            // This check should ensure that only 1 actually successfully does it, b/c the refresh
+            // trigger time will be updated as seen by the subsequent ones. 
+            if (!shouldAttemptRefresh(mgmtData)) {
                 List<EntityDescriptor> descriptors = lookupEntityID(entityID);
                 if (!descriptors.isEmpty()) {
                     log.debug("Metadata was resolved and stored by another thread "