You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2016/02/01 20:16:25 UTC

svn commit: r1727988 - in /sling/trunk/bundles/resourceresolver/src: main/java/org/apache/sling/resourceresolver/impl/ main/java/org/apache/sling/resourceresolver/impl/helper/ test/java/org/apache/sling/resourceresolver/impl/helper/

Author: cziegeler
Date: Mon Feb  1 19:16:25 2016
New Revision: 1727988

URL: http://svn.apache.org/viewvc?rev=1727988&view=rev
Log:
SLING-5470 : Resource providers might not be closed

Modified:
    sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java
    sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContext.java
    sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContextTest.java

Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java?rev=1727988&r1=1727987&r2=1727988&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java (original)
+++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java Mon Feb  1 19:16:25 2016
@@ -29,7 +29,6 @@ import java.util.LinkedList;
 import java.util.Map;
 import java.util.Set;
 import java.util.StringTokenizer;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -98,14 +97,9 @@ public class ResourceResolverImpl extend
     /** The factory which created this resource resolver. */
     private final CommonResourceResolverFactoryImpl factory;
 
-    /** Closed marker. */
-    private final AtomicBoolean isClosed = new AtomicBoolean(false);
-
     /** Resource resolver context. */
     private final ResourceResolverContext context;
 
-    private final Map<String, Object> authenticationInfo;
-
     private volatile Exception closedResolverException;
 
     public ResourceResolverImpl(final CommonResourceResolverFactoryImpl factory, final boolean isAdmin, final Map<String, Object> authenticationInfo) throws LoginException {
@@ -114,8 +108,7 @@ public class ResourceResolverImpl extend
 
     ResourceResolverImpl(final CommonResourceResolverFactoryImpl factory, final boolean isAdmin, final Map<String, Object> authenticationInfo, final ResourceProviderStorage storage) throws LoginException {
         this.factory = factory;
-        this.authenticationInfo = authenticationInfo;
-        this.context = createContext(storage, isAdmin);
+        this.context = createContext(storage, authenticationInfo, isAdmin);
         this.factory.register(this, context);
     }
 
@@ -127,21 +120,21 @@ public class ResourceResolverImpl extend
      */
     private ResourceResolverImpl(final ResourceResolverImpl resolver, final Map<String, Object> authenticationInfo) throws LoginException {
         this.factory = resolver.factory;
-        this.authenticationInfo = new HashMap<String, Object>();
-        if (resolver.authenticationInfo != null) {
-            this.authenticationInfo.putAll(resolver.authenticationInfo);
+        Map<String, Object> authInfo = new HashMap<String, Object>();
+        if (resolver.context.getAuthenticationInfo() != null) {
+            authInfo.putAll(resolver.context.getAuthenticationInfo());
         }
         if (authenticationInfo != null) {
-            this.authenticationInfo.putAll(authenticationInfo);
+            authInfo.putAll(authenticationInfo);
         }
-        this.context = createContext(factory.getResourceProviderTracker().getResourceProviderStorage(), resolver.context.isAdmin());
+        this.context = createContext(factory.getResourceProviderTracker().getResourceProviderStorage(), authInfo, resolver.context.isAdmin());
         this.factory.register(this, context);
     }
 
-    private ResourceResolverContext createContext(ResourceProviderStorage storage, boolean isAdmin)
+    private ResourceResolverContext createContext(ResourceProviderStorage storage, Map<String, Object> authenticationInfo, final boolean isAdmin)
     throws LoginException {
         final ResourceProviderAuthenticator authenticator = new ResourceProviderAuthenticator(this, authenticationInfo, this.factory.getResourceAccessSecurityTracker());
-        final ResourceResolverContext provider = new ResourceResolverContext(isAdmin, storage, this, authenticator);
+        final ResourceResolverContext provider = new ResourceResolverContext(isAdmin, authenticationInfo, storage, this, authenticator);
         authenticator.authenticateAll(storage.getAuthRequiredHandlers(), provider);
         return provider;
     }
@@ -164,7 +157,7 @@ public class ResourceResolverImpl extend
      */
     @Override
     public boolean isLive() {
-        return !this.isClosed.get() && this.context.isLive() && this.factory.isLive();
+        return !this.context.isClosed() && this.context.isLive() && this.factory.isLive();
     }
 
     /**
@@ -175,9 +168,7 @@ public class ResourceResolverImpl extend
         if (factory.shouldLogResourceResolverClosing()) {
             closedResolverException = new Exception("Stack Trace");
         }
-        if ( this.isClosed.compareAndSet(false, true)) {
-            this.factory.unregister(this, this.context);
-        }
+        this.factory.unregister(this, this.context);
     }
 
     /**
@@ -187,7 +178,7 @@ public class ResourceResolverImpl extend
      *             If the resolver is already closed or the factory is no longer live.
      */
     private void checkClosed() {
-        if (this.isClosed.get()) {
+        if (this.context.isClosed()) {
             if (closedResolverException != null) {
                 logger.error("The ResourceResolver has already been closed.", closedResolverException);
             }
@@ -763,12 +754,12 @@ public class ResourceResolverImpl extend
         checkClosed();
 
         // Try auth info first
-        if ( this.authenticationInfo != null ) {
-            final Object impUser = this.authenticationInfo.get(ResourceResolverFactory.USER_IMPERSONATION);
+        if ( this.context.getAuthenticationInfo() != null ) {
+            final Object impUser = this.context.getAuthenticationInfo().get(ResourceResolverFactory.USER_IMPERSONATION);
             if ( impUser != null ) {
                 return impUser.toString();
             }
-            final Object user = this.authenticationInfo.get(ResourceResolverFactory.USER);
+            final Object user = this.context.getAuthenticationInfo().get(ResourceResolverFactory.USER);
             if ( user != null ) {
                 return user.toString();
             }

Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContext.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContext.java?rev=1727988&r1=1727987&r2=1727988&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContext.java (original)
+++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContext.java Mon Feb  1 19:16:25 2016
@@ -74,7 +74,7 @@ public class ResourceResolverContext {
     private final boolean isAdmin;
 
     /** Resource type resource resolver (admin resolver) */
-    private ResourceResolver resourceTypeResourceResolver;
+    private volatile ResourceResolver resourceTypeResourceResolver;
 
     /** Flag for handling multiple calls to close. */
     private final AtomicBoolean isClosed = new AtomicBoolean(false);
@@ -85,13 +85,17 @@ public class ResourceResolverContext {
 
     private final ResourceProviderAuthenticator authenticator;
 
+    private final Map<String, Object> authenticationInfo;
+
     /**
      * Create a new resource resolver context.
      */
     public ResourceResolverContext(final boolean isAdmin,
+            final Map<String, Object> authenticationInfo,
             ResourceProviderStorage storage,
             ResourceResolver resolver,
             ResourceProviderAuthenticator authenticator) {
+        this.authenticationInfo = authenticationInfo;
         this.isAdmin = isAdmin;
         this.storage = storage;
         this.resolver = resolver;
@@ -102,6 +106,13 @@ public class ResourceResolverContext {
         return isAdmin;
     }
 
+    public Map<String, Object> getAuthenticationInfo() {
+        return this.authenticationInfo;
+    }
+
+    public boolean isClosed() {
+        return this.isClosed.get();
+    }
     /**
      * Logs out from all providers.
      */

Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContextTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContextTest.java?rev=1727988&r1=1727987&r2=1727988&view=diff
==============================================================================
--- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContextTest.java (original)
+++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverContextTest.java Mon Feb  1 19:16:25 2016
@@ -141,7 +141,7 @@ public class ResourceResolverContextTest
         ResourceProviderStorage storage = new ResourceProviderStorage(handlers);
         authenticator = new ResourceProviderAuthenticator(rr, authInfo, securityTracker);
 
-        crp = new ResourceResolverContext(false, storage, rr, authenticator);
+        crp = new ResourceResolverContext(false, authInfo, storage, rr, authenticator);
     }
 
     /**