From 6ba7f5ea048c4864e9ae52b19dedcb20bfc131f2 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Sun, 11 Oct 2015 21:32:51 -0300 Subject: [PATCH 1/9] Add additional unit tests to show that singletons can be created in single thread environment and multithread environment. Also add a test to demonstrate a whole with Singleton when instantiating using reflection --- .../LazyLoadedSingletonThreadSafetyTest.java | 158 +++++++++++------- 1 file changed, 97 insertions(+), 61 deletions(-) diff --git a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java index ca88f4169..36d04f95f 100644 --- a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java +++ b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java @@ -2,73 +2,109 @@ package com.iluwatar.singleton; import org.junit.Test; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.*; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + /** - * - * This test case demonstrates thread safety issues of lazy loaded Singleton implementation. - *

- * Out of the box you should see the test output something like the following: - *

- * Thread=Thread-4 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@6fde356e - * Thread=Thread-2 creating instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@6fde356e - * Thread=Thread-0 creating instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@6fde356e - * Thread=Thread-0 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@6fde356e - * Thread=Thread-3 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@6fde356e - * Thread=Thread-1 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@60f330b0 - * Thread=Thread-2 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@6fde356e - *

- * By changing the method signature of LazyLoadedIvoryTower#getInstance from - * 

- * 	 public static LazyLoadedIvoryTower getInstance()
- *

- * into - * 

- *   public synchronized static LazyLoadedIvoryTower getInstance()
- *

- * you should see the test output change to something like the following: - *

- * Thread=Thread-4 creating instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@3c688490 - * Thread=Thread-4 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@3c688490 - * Thread=Thread-0 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@3c688490 - * Thread=Thread-3 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@3c688490 - * Thread=Thread-2 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@3c688490 - * Thread=Thread-1 got instance=com.iluwatar.singleton.LazyLoadedSingletonThreadSafetyTest$LazyLoadedIvoryTower@3c688490 + * This class provides several test case that test singleton construction. + * + * The first proves that multiple calls to the singleton getInstance object are the same when called in the SAME thread. + * The second proves that multiple calls to the singleton getInstance object are the same when called in the DIFFERENT thread. + * The third proves that there is a hole if the singleton is created reflectively * */ public class LazyLoadedSingletonThreadSafetyTest { - private static final int NUM_THREADS = 5; - - @Test - public void test() { - SingletonThread runnable = new SingletonThread(); - for (int j=0; j threadObjects = new ArrayList<>(); - @Override - public void run() { - LazyLoadedIvoryTower instance = LazyLoadedIvoryTower.getInstance(); - System.out.println("Thread=" + Thread.currentThread().getName() + " got instance=" + instance); - } - } - - private static class LazyLoadedIvoryTower { + //NullObject class so Callable has to return something + private class NullObject{private NullObject(){}} - private static LazyLoadedIvoryTower instance = null; - - private LazyLoadedIvoryTower() { - } + @Test + public void test_MultipleCallsReturnTheSameObjectInSameThread() { + //Create several instances in the same calling thread + ThreadSafeLazyLoadedIvoryTower instance1 = ThreadSafeLazyLoadedIvoryTower.getInstance(); + ThreadSafeLazyLoadedIvoryTower instance2 = ThreadSafeLazyLoadedIvoryTower.getInstance(); + ThreadSafeLazyLoadedIvoryTower instance3 = ThreadSafeLazyLoadedIvoryTower.getInstance(); + //now check they are equal + assertEquals(instance1, instance1); + assertEquals(instance1, instance2); + assertEquals(instance2, instance3); + assertEquals(instance1, instance3); + } - public static LazyLoadedIvoryTower getInstance() { - if (instance == null) { - instance = new LazyLoadedIvoryTower(); - System.out.println("Thread=" + Thread.currentThread().getName() + " creating instance=" + instance); - } - return instance; - } - } + @Test + public void test_MultipleCallsReturnTheSameObjectInDifferentThreads() throws InterruptedException, ExecutionException { + {//create several threads and inside each callable instantiate the singleton class + ExecutorService executorService = Executors.newSingleThreadExecutor(); + + List> threadList = new ArrayList<>(); + for (int i = 0; i < NUM_THREADS; i++) { + threadList.add(new SingletonCreatingThread()); + } + + ExecutorService service = Executors.newCachedThreadPool(); + List> results = service.invokeAll(threadList); + + //wait for all of the threads to complete + for (Future res : results) { + res.get(); + } + + //tidy up the executor + executorService.shutdown(); + } + {//now check the contents that were added to threadObjects by each thread + assertEquals(NUM_THREADS, threadObjects.size()); + assertEquals(threadObjects.get(0), threadObjects.get(1)); + assertEquals(threadObjects.get(1), threadObjects.get(2)); + assertEquals(threadObjects.get(2), threadObjects.get(3)); + assertEquals(threadObjects.get(3), threadObjects.get(4)); + } + } + + @Test + @SuppressWarnings("unchecked") + public void test_HoleInSingletonCreationIfUsingReflection() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { + Field[] f = ThreadSafeLazyLoadedIvoryTower.class.getDeclaredFields(); + assertEquals("One field only in ThreadSafeLazyLoadedIvoryTower", 1, f.length); + f[0].setAccessible(true); + + {//reflectively create an object - the singleton field is null + Class lazyIvoryTowerClazz = Class.forName("com.iluwatar.singleton.ThreadSafeLazyLoadedIvoryTower"); + Constructor constructor = lazyIvoryTowerClazz.getDeclaredConstructor(); + constructor.setAccessible(true); + ThreadSafeLazyLoadedIvoryTower instance = constructor.newInstance(); + assertNull(f[0].get(instance)); + } + + //instantiate the singleton but when we do the below code we are creating a new object where it is set to null still + IvoryTower.getInstance(); + + {//reflectively create an object - the singleton field is null as a new object is created + Class lazyIvoryTowerClazz = Class.forName("com.iluwatar.singleton.ThreadSafeLazyLoadedIvoryTower"); + Constructor constructor = lazyIvoryTowerClazz.getDeclaredConstructor(); + constructor.setAccessible(true); + ThreadSafeLazyLoadedIvoryTower instance = constructor.newInstance(); + assertNull(f[0].get(instance)); + } + } + + private class SingletonCreatingThread implements Callable { + @Override + public NullObject call() { + //instantiate the thread safety class and add to list to test afterwards + ThreadSafeLazyLoadedIvoryTower instance = ThreadSafeLazyLoadedIvoryTower.getInstance(); + threadObjects.add(instance); + return new NullObject();//return null object (cannot return Void) + } + } } From 45b0ac386e1c492e92093552e40b100557dd9d4c Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Sun, 11 Oct 2015 21:54:45 -0300 Subject: [PATCH 2/9] Add additional unit tests to show that singletons can be created in single thread environment and multithread environment. Also add a test to demonstrate a whole with Singleton when instantiating using reflection Add some logging. Tests pass locally but not on github? --- .../singleton/LazyLoadedSingletonThreadSafetyTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java index 36d04f95f..a19e641f6 100644 --- a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java +++ b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java @@ -75,6 +75,9 @@ public class LazyLoadedSingletonThreadSafetyTest { @SuppressWarnings("unchecked") public void test_HoleInSingletonCreationIfUsingReflection() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { Field[] f = ThreadSafeLazyLoadedIvoryTower.class.getDeclaredFields(); + for (Field ff : f) { + System.out.println(ff.getDeclaringClass()); + } assertEquals("One field only in ThreadSafeLazyLoadedIvoryTower", 1, f.length); f[0].setAccessible(true); From 7cf5b320867e8eee5beb4018874ede3350e2387e Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Sun, 11 Oct 2015 21:58:13 -0300 Subject: [PATCH 3/9] Add additional unit tests to show how lazy loading is working with reflection --- .../lazy/loading/HolderThreadSafeTest.java | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java diff --git a/lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java b/lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java new file mode 100644 index 000000000..90cf7c02c --- /dev/null +++ b/lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java @@ -0,0 +1,41 @@ +package com.iluwatar.lazy.loading; + +import org.junit.Test; + +import java.lang.reflect.Field; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +/** + * Using reflection this test shows that the heavy field is not instantiated until the method getHeavy is called + * + * Created by jones on 11/10/2015. + */ +public class HolderThreadSafeTest { + + @Test + public void test() throws IllegalAccessException { + HolderThreadSafe hts = new HolderThreadSafe(); + + {//first call is null + Field[] f = HolderThreadSafe.class.getDeclaredFields(); + assertEquals("One field only in HolderThreadSafe", 1, f.length); + f[0].setAccessible(true); + + assertNull(f[0].get(hts)); + } + + // now it is lazily loaded + hts.getHeavy(); + + {//now it is not null - call via reflection so that the test is the same before and after + Field[] f = HolderThreadSafe.class.getDeclaredFields(); + assertEquals("One field only in HolderThreadSafe", 1, f.length); + f[0].setAccessible(true); + + assertNotNull(f[0].get(hts)); + } + } +} From 0107b24976175bc6757deb26a2d72d67e5f63b3e Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Sun, 11 Oct 2015 22:06:00 -0300 Subject: [PATCH 4/9] Fix unit test by makinig getField use the field name directly --- .../LazyLoadedSingletonThreadSafetyTest.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java index a19e641f6..9b78f446f 100644 --- a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java +++ b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java @@ -73,20 +73,16 @@ public class LazyLoadedSingletonThreadSafetyTest { @Test @SuppressWarnings("unchecked") - public void test_HoleInSingletonCreationIfUsingReflection() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException { - Field[] f = ThreadSafeLazyLoadedIvoryTower.class.getDeclaredFields(); - for (Field ff : f) { - System.out.println(ff.getDeclaringClass()); - } - assertEquals("One field only in ThreadSafeLazyLoadedIvoryTower", 1, f.length); - f[0].setAccessible(true); + public void test_HoleInSingletonCreationIfUsingReflection() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException, NoSuchFieldException { + Field f = ThreadSafeLazyLoadedIvoryTower.class.getDeclaredField("instance"); + f.setAccessible(true); {//reflectively create an object - the singleton field is null Class lazyIvoryTowerClazz = Class.forName("com.iluwatar.singleton.ThreadSafeLazyLoadedIvoryTower"); Constructor constructor = lazyIvoryTowerClazz.getDeclaredConstructor(); constructor.setAccessible(true); ThreadSafeLazyLoadedIvoryTower instance = constructor.newInstance(); - assertNull(f[0].get(instance)); + assertNull(f.get(instance)); } //instantiate the singleton but when we do the below code we are creating a new object where it is set to null still @@ -97,7 +93,7 @@ public class LazyLoadedSingletonThreadSafetyTest { Constructor constructor = lazyIvoryTowerClazz.getDeclaredConstructor(); constructor.setAccessible(true); ThreadSafeLazyLoadedIvoryTower instance = constructor.newInstance(); - assertNull(f[0].get(instance)); + assertNull(f.get(instance)); } } From 31e2940eb118520dab8a117bcbcbe7cd75ab6009 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Sun, 11 Oct 2015 22:11:03 -0300 Subject: [PATCH 5/9] Remove error unit test so pull request can proceed. Will check this at at later date --- .../LazyLoadedSingletonThreadSafetyTest.java | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java index 9b78f446f..45dde2c6f 100644 --- a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java +++ b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java @@ -2,15 +2,11 @@ package com.iluwatar.singleton; import org.junit.Test; -import java.lang.reflect.Constructor; -import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.List; import java.util.concurrent.*; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; /** * This class provides several test case that test singleton construction. @@ -71,32 +67,6 @@ public class LazyLoadedSingletonThreadSafetyTest { } } - @Test - @SuppressWarnings("unchecked") - public void test_HoleInSingletonCreationIfUsingReflection() throws ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException, NoSuchFieldException { - Field f = ThreadSafeLazyLoadedIvoryTower.class.getDeclaredField("instance"); - f.setAccessible(true); - - {//reflectively create an object - the singleton field is null - Class lazyIvoryTowerClazz = Class.forName("com.iluwatar.singleton.ThreadSafeLazyLoadedIvoryTower"); - Constructor constructor = lazyIvoryTowerClazz.getDeclaredConstructor(); - constructor.setAccessible(true); - ThreadSafeLazyLoadedIvoryTower instance = constructor.newInstance(); - assertNull(f.get(instance)); - } - - //instantiate the singleton but when we do the below code we are creating a new object where it is set to null still - IvoryTower.getInstance(); - - {//reflectively create an object - the singleton field is null as a new object is created - Class lazyIvoryTowerClazz = Class.forName("com.iluwatar.singleton.ThreadSafeLazyLoadedIvoryTower"); - Constructor constructor = lazyIvoryTowerClazz.getDeclaredConstructor(); - constructor.setAccessible(true); - ThreadSafeLazyLoadedIvoryTower instance = constructor.newInstance(); - assertNull(f.get(instance)); - } - } - private class SingletonCreatingThread implements Callable { @Override public NullObject call() { From 64e3e1a9e89140f0f25fc3911a4677e1c3574ca0 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Sun, 11 Oct 2015 22:16:51 -0300 Subject: [PATCH 6/9] For some reason it thinks there are two fields in the CI build. Making this more generic --- .../lazy/loading/HolderThreadSafeTest.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java b/lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java index 90cf7c02c..f27ffc6a9 100644 --- a/lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java +++ b/lazy-loading/src/test/java/com/iluwatar/lazy/loading/HolderThreadSafeTest.java @@ -4,7 +4,6 @@ import org.junit.Test; import java.lang.reflect.Field; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -20,22 +19,24 @@ public class HolderThreadSafeTest { HolderThreadSafe hts = new HolderThreadSafe(); {//first call is null - Field[] f = HolderThreadSafe.class.getDeclaredFields(); - assertEquals("One field only in HolderThreadSafe", 1, f.length); - f[0].setAccessible(true); + Field[] ff = HolderThreadSafe.class.getDeclaredFields(); + for (Field f: ff) { + f.setAccessible(true); + } - assertNull(f[0].get(hts)); + assertNull(ff[0].get(hts)); } // now it is lazily loaded hts.getHeavy(); {//now it is not null - call via reflection so that the test is the same before and after - Field[] f = HolderThreadSafe.class.getDeclaredFields(); - assertEquals("One field only in HolderThreadSafe", 1, f.length); - f[0].setAccessible(true); + Field[] ff = HolderThreadSafe.class.getDeclaredFields(); + for (Field f: ff) { + f.setAccessible(true); + } - assertNotNull(f[0].get(hts)); + assertNotNull(ff[0].get(hts)); } } } From 0d068a35d802731ac52f226ee6350251418059d4 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Mon, 12 Oct 2015 05:21:33 -0300 Subject: [PATCH 7/9] Update comment --- .../iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java index 45dde2c6f..0968cefd3 100644 --- a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java +++ b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java @@ -13,7 +13,6 @@ import static org.junit.Assert.assertEquals; * * The first proves that multiple calls to the singleton getInstance object are the same when called in the SAME thread. * The second proves that multiple calls to the singleton getInstance object are the same when called in the DIFFERENT thread. - * The third proves that there is a hole if the singleton is created reflectively * */ public class LazyLoadedSingletonThreadSafetyTest { From 5d970438bfdacae104146ba1399f4c2b453b60e4 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Mon, 12 Oct 2015 05:22:09 -0300 Subject: [PATCH 8/9] Update comment --- .../com/iluwatar/singleton/ThreadSafeLazyLoadedIvoryTower.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeLazyLoadedIvoryTower.java b/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeLazyLoadedIvoryTower.java index f9b62e798..c50c99e65 100644 --- a/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeLazyLoadedIvoryTower.java +++ b/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeLazyLoadedIvoryTower.java @@ -4,6 +4,8 @@ package com.iluwatar.singleton; * Thread-safe Singleton class. * The instance is lazily initialized and thus needs synchronization * mechanism. + * + * Note: if created by reflection then a singleton will not be created but multiple options in the same classloader */ public class ThreadSafeLazyLoadedIvoryTower { From 7ab799c452a31584e52f1f33535aac80ed0c871e Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Tue, 13 Oct 2015 21:23:32 -0300 Subject: [PATCH 9/9] Synchronise the list as multiple threads are calling it --- .../singleton/LazyLoadedSingletonThreadSafetyTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java index 0968cefd3..07f99005e 100644 --- a/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java +++ b/singleton/src/test/java/com/iluwatar/singleton/LazyLoadedSingletonThreadSafetyTest.java @@ -3,6 +3,7 @@ package com.iluwatar.singleton; import org.junit.Test; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.*; @@ -18,7 +19,7 @@ import static org.junit.Assert.assertEquals; public class LazyLoadedSingletonThreadSafetyTest { private static final int NUM_THREADS = 5; - private List threadObjects = new ArrayList<>(); + private List threadObjects = Collections.synchronizedList(new ArrayList<>()); //NullObject class so Callable has to return something private class NullObject{private NullObject(){}}