Author: radu Date: Sat Oct 3 13:00:31 2015 New Revision: 1706574 URL: http://svn.apache.org/viewvc?rev=1706574&view=rev Log: SLING-5087 - The ResourceResolverImpl should provide a stack trace when CRUD operations are performed with a closed resolver
* added option to toggle logging; by default the logging is off Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java?rev=1706574&r1=1706573&r2=1706574&view=diff ============================================================================== --- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java (original) +++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java Sat Oct 3 13:00:31 2015 @@ -79,11 +79,14 @@ public class CommonResourceResolverFacto /** Background thread handling disposing of resource resolver instances. */ private final Thread refQueueThread; + private boolean logResourceResolverClosing = false; + /** * Create a new common resource resolver factory. */ public CommonResourceResolverFactoryImpl(final ResourceResolverFactoryActivator activator) { this.activator = activator; + this.logResourceResolverClosing = activator.shouldLogResourceResolverClosing(); this.refQueueThread = new Thread("Apache Sling Resource Resolver Finalizer Thread") { @Override @@ -206,7 +209,7 @@ public class CommonResourceResolverFacto /** * Inform about a closed resource resolver. * Make sure to remove it from the current thread context. - * @param resolver The resource resolver + * @param resourceResolverImpl The resource resolver * @param ctx The resource resolver context */ public void unregister(final ResourceResolver resourceResolverImpl, @@ -407,6 +410,10 @@ public class CommonResourceResolverFacto return this.isActive.get(); } + public boolean shouldLogResourceResolverClosing() { + return logResourceResolverClosing; + } + /** * Extension of a weak reference to be able to get the context object * that is used for cleaning up. Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java?rev=1706574&r1=1706573&r2=1706574&view=diff ============================================================================== --- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java (original) +++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java Sat Oct 3 13:00:31 2015 @@ -243,6 +243,14 @@ public class ResourceResolverFactoryActi + "for memory leaks caused by objects hold from that resource provider.") private static final String PROP_PARANOID_PROVIDER_HANDLING = "resource.resolver.providerhandling.paranoid"; + private static final boolean DEFAULT_LOG_RESOURCE_RESOLVER_CLOSING = false; + @Property(boolValue = DEFAULT_LOG_RESOURCE_RESOLVER_CLOSING, + label = "Log resource resolver closing", + description = "When enabled CRUD operations with a closed resource resolver will log a stack trace " + + "with the point where the used resolver was closed. It's advisable to not enable this feature on " + + "production systems.") + private static final String PROP_LOG_RESOURCE_RESOLVER_CLOSING = "resource.resolver.log.closing"; + /** Tracker for the resource decorators. */ private final ResourceDecoratorTracker resourceDecoratorTracker = new ResourceDecoratorTracker(); @@ -301,6 +309,9 @@ public class ResourceResolverFactoryActi /** vanity paths will have precedence over existing /etc/map mapping? */ private boolean vanityPathPrecedence = DEFAULT_VANITY_PATH_PRECEDENCE; + /** log the place where a resource resolver is closed */ + private boolean logResourceResolverClosing = DEFAULT_LOG_RESOURCE_RESOLVER_CLOSING; + /** Vanity path whitelist */ private String[] vanityPathWhiteList; @@ -413,6 +424,10 @@ public class ResourceResolverFactoryActi return this.vanityBloomFilterMaxBytes; } + public boolean shouldLogResourceResolverClosing() { + return logResourceResolverClosing; + } + // ---------- SCR Integration --------------------------------------------- /** @@ -521,6 +536,8 @@ public class ResourceResolverFactoryActi this.vanityBloomFilterMaxBytes = PropertiesUtil.toInteger(properties.get(PROP_VANITY_BLOOM_FILTER_MAX_BYTES), DEFAULT_VANITY_BLOOM_FILTER_MAX_BYTES); this.vanityPathPrecedence = PropertiesUtil.toBoolean(properties.get(PROP_VANITY_PATH_PRECEDENCE), DEFAULT_VANITY_PATH_PRECEDENCE); + this.logResourceResolverClosing = PropertiesUtil.toBoolean(properties.get(PROP_LOG_RESOURCE_RESOLVER_CLOSING), + DEFAULT_LOG_RESOURCE_RESOLVER_CLOSING); final BundleContext bc = componentContext.getBundleContext(); 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=1706574&r1=1706573&r2=1706574&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 Sat Oct 3 13:00:31 2015 @@ -100,7 +100,7 @@ public class ResourceResolverImpl extend /** Resource resolver context. */ private final ResourceResolverContext context; - private Exception closedResolverException; + private volatile Exception closedResolverException; /** * The resource resolver context. @@ -152,7 +152,9 @@ public class ResourceResolverImpl extend */ @Override public void close() { - closedResolverException = new Exception("Stack Trace"); + if (factory.shouldLogResourceResolverClosing()) { + closedResolverException = new Exception("Stack Trace"); + } if ( this.isClosed.compareAndSet(false, true)) { this.factory.unregister(this, this.context); } Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java?rev=1706574&r1=1706573&r2=1706574&view=diff ============================================================================== --- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java (original) +++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java Sat Oct 3 13:00:31 2015 @@ -46,6 +46,7 @@ import org.apache.sling.resourceresolver import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; +import org.mockito.internal.util.reflection.Whitebox; public class ResourceResolverImplTest { @@ -66,6 +67,118 @@ public class ResourceResolverImplTest { assertTrue(rr.isLive()); rr.close(); assertFalse(rr.isLive()); + // close is always allowed to be called + rr.close(); + assertFalse(rr.isLive()); + // now check all public method - they should all throw! + try { + rr.adaptTo(Session.class); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.clone(null); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.findResources("a", "b"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.getAttribute("a"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.getAttributeNames(); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.getResource(null); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.getResource(null, "/a"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.getSearchPath(); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.getUserID(); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.listChildren(null); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.map("/somepath"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.map(null, "/somepath"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.queryResources("a", "b"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.resolve((HttpServletRequest)null); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.resolve("/path"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + try { + rr.resolve(null, "/path"); + fail(); + } catch (final IllegalStateException ise) { + // expected + } + } + + @Test + public void testCloseWithStackTraceLogging() throws Exception { + ResourceResolverFactoryActivator rrfa = Mockito.spy(new ResourceResolverFactoryActivator()); + Whitebox.setInternalState(rrfa, "logResourceResolverClosing", true); + CommonResourceResolverFactoryImpl crrfi = new CommonResourceResolverFactoryImpl(rrfa); + final ResourceResolver rr = new ResourceResolverImpl(crrfi, new ResourceResolverContext(false, null, new + ResourceAccessSecurityTracker())); + assertTrue(rr.isLive()); + rr.close(); + assertFalse(rr.isLive()); // close is always allowed to be called rr.close(); assertFalse(rr.isLive());