You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2015/10/03 15:00:32 UTC
svn commit: r1706574 - in /sling/trunk/bundles/resourceresolver/src:
main/java/org/apache/sling/resourceresolver/impl/
test/java/org/apache/sling/resourceresolver/impl/
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());