[sling-org-apache-sling-models-impl] branch master updated: SLING-7470 - resolving memory leak for fake request objects

[ju...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

justin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-models-impl.git


The following commit(s) were added to refs/heads/master by this push:
     new b0647a3  SLING-7470 - resolving memory leak for fake request objects
b0647a3 is described below

commit b0647a3419924c46e58b78aa6384e0f49491a0c6
Author: Justin Edelson <ju...@apache.org>
AuthorDate: Fri Feb 2 19:13:25 2018 -0500

    SLING-7470 - resolving memory leak for fake request objects
---
 .../sling/models/impl/ModelAdapterFactory.java     |  8 +++-
 .../sling/models/impl/RequestDisposalTest.java     | 45 +++++++++++++++++++++-
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
index 9b1bdc1..9fcdb12 100644
--- a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
+++ b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
@@ -132,6 +132,10 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
     // hard code this value since we always know exactly how many there are
     private static final int VALUE_PREPARERS_COUNT = 2;
 
+    private static final String REQUEST_MARKER_ATTRIBUTE = ModelAdapterFactory.class.getName() + ".RealRequest";
+
+    private static final Object REQUEST_MARKER_VALUE = new Object();
+
     private static class DisposalCallbackRegistryImpl implements DisposalCallbackRegistry {
 
         private List<DisposalCallback> callbacks = new ArrayList<>();
@@ -684,7 +688,8 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
         if (!registry.callbacks.isEmpty()) {
             registry.seal();
 
-            if (adaptable instanceof SlingHttpServletRequest) {
+            if (adaptable instanceof SlingHttpServletRequest &&
+                    ((SlingHttpServletRequest) adaptable).getAttribute(REQUEST_MARKER_ATTRIBUTE) == REQUEST_MARKER_VALUE) {
                 registerRequestCallbackRegistry((SlingHttpServletRequest) adaptable, registry);
             } else {
                 registerCallbackRegistry(object, registry);
@@ -1314,5 +1319,6 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
 
     @Override
     public void requestInitialized(ServletRequestEvent sre) {
+        sre.getServletRequest().setAttribute(REQUEST_MARKER_ATTRIBUTE, REQUEST_MARKER_VALUE);
     }
 }
diff --git a/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java b/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
index c163798..b0c30a9 100644
--- a/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
+++ b/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
@@ -27,7 +27,9 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.stubbing.Answer;
 import org.osgi.framework.BundleContext;
 import org.osgi.service.component.ComponentContext;
 
@@ -38,7 +40,9 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletRequestEvent;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Type;
+import java.util.HashMap;
 import java.util.Hashtable;
+import java.util.Map;
 
 import static org.mockito.Mockito.*;
 import static org.junit.Assert.*;
@@ -74,14 +78,33 @@ public class RequestDisposalTest {
         factory.bindInjector(new DisposedInjector(), new ServicePropertiesMap(0, 0));
         factory.adapterImplementations.addClassesAsAdapterAndImplementation(TestModel.class);
 
-        callback = new TestDisposalCallback();
+        final Map<String, Object> attributes = new HashMap<>();
+
+        doAnswer(new Answer<Void>() {
+
+            @Override
+            public Void answer(InvocationOnMock invocation) {
+                attributes.put((String) invocation.getArguments()[0], invocation.getArguments()[1]);
+                return null;
+            }
+        }).when(request).setAttribute(any(String.class), any());
 
+        when(request.getAttribute(any(String.class))).then(new Answer<Object>() {
+
+            @Override
+            public Object answer(InvocationOnMock invocation) {
+                return attributes.get(invocation.getArguments()[0]);
+            }
+        });
+
+        callback = new TestDisposalCallback();
     }
 
     @Test
-    public void test() {
+    public void testWithInitializedRequest() {
         // destroy a wrapper of the root request
         SlingHttpServletRequest destroyedRequest = new SlingHttpServletRequestWrapper(request);
+        factory.requestInitialized(new ServletRequestEvent(servletContext, destroyedRequest));
 
         // but adapt from a wrapper of a wrapper of that wrapper
         SlingHttpServletRequest adaptableRequest = new SlingHttpServletRequestWrapper(new SlingHttpServletRequestWrapper(destroyedRequest));
@@ -96,6 +119,24 @@ public class RequestDisposalTest {
         assertTrue(callback.isDisposed());
     }
 
+    @Test
+    public void testWithUnitializedRequest() {
+        // destroy a wrapper of the root request
+        SlingHttpServletRequest destroyedRequest = new SlingHttpServletRequestWrapper(request);
+
+        // but adapt from a wrapper of a wrapper of that wrapper
+        SlingHttpServletRequest adaptableRequest = new SlingHttpServletRequestWrapper(new SlingHttpServletRequestWrapper(destroyedRequest));
+
+        TestModel model = factory.getAdapter(adaptableRequest, TestModel.class);
+        assertEquals("teststring", model.testString);
+
+        assertFalse(callback.isDisposed());
+
+        factory.requestDestroyed(new ServletRequestEvent(servletContext, destroyedRequest));
+
+        assertFalse(callback.isDisposed());
+    }
+
     @Model(adaptables = SlingHttpServletRequest.class)
     public static class TestModel {
 

-- 
To stop receiving notification emails like this one, please contact
justin@apache.org.