SLING-6584 - synchronize setAccessible calls
authorJustin Edelson <justin@apache.org>
Wed, 1 Mar 2017 14:58:55 +0000 (14:58 +0000)
committerJustin Edelson <justin@apache.org>
Wed, 1 Mar 2017 14:58:55 +0000 (14:58 +0000)
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1784960 13f79535-47bb-0310-9956-ffa450edef68

pom.xml
src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
src/main/java/org/apache/sling/models/impl/model/InjectableField.java
src/test/java/org/apache/sling/models/impl/ConstructorTest.java

diff --git a/pom.xml b/pom.xml
index a059ea1..7e30f37 100644 (file)
--- a/pom.xml
+++ b/pom.xml
         <dependency>\r
             <groupId>org.apache.sling</groupId>\r
             <artifactId>org.apache.sling.models.api</artifactId>\r
-            <version>1.3.3-SNAPSHOT</version>\r
+            <version>1.3.2</version>\r
             <scope>provided</scope>\r
         </dependency>\r
         <dependency>\r
index 35024e5..a31e02f 100644 (file)
@@ -799,23 +799,9 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
     }
 
     private RuntimeException setField(InjectableField injectableField, Object createdObject, Object value) {
-        Field field = injectableField.getField();
-        Result<Object> result = adaptIfNecessary(value, field.getType(), field.getGenericType());
+        Result<Object> result = adaptIfNecessary(value, injectableField.getFieldType(), injectableField.getFieldGenericType());
         if (result.wasSuccessful()) {
-            boolean accessible = field.isAccessible();
-            try {
-                if (!accessible) {
-                    field.setAccessible(true);
-                }
-                field.set(createdObject, result.getValue());
-            } catch (Exception e) {
-                return new ModelClassException("Could not inject field due to reflection issues", e);
-            } finally {
-                if (!accessible) {
-                    field.setAccessible(false);
-                }
-            }
-            return null;
+            return injectableField.set(createdObject, result);
         } else {
             return result.getThrowable();
         }
index 7deef40..ccf17d6 100644 (file)
 package org.apache.sling.models.impl.model;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Type;
 
 import org.apache.sling.models.annotations.DefaultInjectionStrategy;
+import org.apache.sling.models.factory.ModelClassException;
 import org.apache.sling.models.impl.ReflectionUtil;
+import org.apache.sling.models.impl.Result;
 import org.apache.sling.models.spi.injectorspecific.StaticInjectAnnotationProcessorFactory;
 
 public class InjectableField extends AbstractInjectableElement {
@@ -32,13 +35,35 @@ public class InjectableField extends AbstractInjectableElement {
         super(field, ReflectionUtil.mapPrimitiveClasses(field.getGenericType()), field.getName(), processorFactories, defaultInjectionStrategy);
         this.field = field;
     }
-    
-    public Field getField() {
-        return field;
+
+    public RuntimeException set(Object createdObject, Result<Object> result) {
+        synchronized (field) {
+            boolean accessible = field.isAccessible();
+            try {
+                if (!accessible) {
+                    field.setAccessible(true);
+                }
+                field.set(createdObject, result.getValue());
+            } catch (Exception e) {
+                return new ModelClassException("Could not inject field due to reflection issues", e);
+            } finally {
+                if (!accessible) {
+                    field.setAccessible(false);
+                }
+            }
+        }
+        return null;
     }
 
     public boolean isPrimitive() {
         return false;
     }
 
+    public Class<?> getFieldType() {
+        return field.getType();
+    }
+
+    public Type getFieldGenericType() {
+        return field.getGenericType();
+    }
 }
index 549c18d..3393f01 100644 (file)
@@ -19,7 +19,14 @@ package org.apache.sling.models.impl;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
+import java.util.ArrayList;
 import java.util.Hashtable;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.models.factory.ModelClassException;
@@ -131,6 +138,49 @@ public class ConstructorTest {
     }
 
     @Test
+    public void testMultiThreadedConstructorInjection() throws InterruptedException, ExecutionException {
+
+        class ModelCreator implements Callable<String> {
+            @Override
+            public String call() throws Exception {
+                try {
+                    WithOneConstructorModel model = factory.getAdapter(request, WithOneConstructorModel.class);
+                    if (model == null) {
+                        return "Expected model not null";
+                    }
+                    if (!request.equals(model.getRequest())) {
+                        return "Expected same request";
+                    }
+                    if (INT_VALUE != model.getAttribute()) {
+                        return "Expected same value for attribute";
+                    }
+                } catch (Throwable e) {
+                    return "Exception not expected: " + e;
+                }
+                return null;
+            }
+        }
+
+        int tries = 10000;
+        int threads = 10;
+
+        ExecutorService executorService = Executors.newFixedThreadPool(threads);
+        List<ModelCreator> modelCreators = new ArrayList<>(tries);
+        for (int i = 0; i < tries; i++) {
+            modelCreators.add(new ModelCreator());
+        }
+        List<Future<String>> futures = executorService.invokeAll(modelCreators);
+        executorService.shutdown();
+
+        for (Future<String> f : futures) {
+            String res = f.get();
+            if (res != null) {
+                fail(res);
+            }
+        }
+    }
+
+    @Test
     public void testNoNameModel() {
         NoNameModel model = factory.getAdapter(request, NoNameModel.class);
         assertNull(model);