Ensure singleton object creation thread safety

This fixes possible race conditions creating the singleton instances of:

- `BeaconManager`
- `DetectionTracker`
- `MonitoringStatus`
- `Stats`

Both `DetectionTracker` and `Stats` require no argument to their
constructors. The singleton instances are also both `private` to the
class. This means we can create the singleton instance as a constant
without worry about any additional threading issues; Java guarantees
constant creation thread safety (see _Effective Java_, Second Edition,
_Item 3: Enforce the singleton property with a private constructor or an
enum type_).

However, both `BeaconManager` and `MonitoringStatus` require the
application context in order to be created. This means we cannot use a
constant since we won't have the application context instance available
at constant creation time. Instead this uses the lazy-loading double
check locking pattern with a private lock object. While this double
check locking idiom may seem a bit odd it is a well published and tested
pattern. The source of the idiom is cited in the local code comments. We
also change the instances to be `volatile` to ensure that any thread
that reads the field will see the most recently written value - which is
necessary since there is no locking when we have already initialized the
field. For more details on this pattern see _Effective Java_, Second
Edition, _Item 71: Use lazy initialization judiciously_.

This additionally fixes a possible denial-of-service condition with the
previous `MonitoringStatus` singleton implementation. This DOS issue can
occur because the synchronization is performed by locking the externally
accessible `MonitoringStatus.class` object. An external thread can also
attempt to lock on the class object which could lead to a dead-lock
situation. While it's unlikely it's still possible and this new
implementation guarantees that won't happen due to the use of the
private lock object (see _Effective Java_, Second Edition, _Item 70:
Document thread safety_).

References:

Joshua Bloch. _Effective Java_, Second Edition. Addison-Wesley, 2008

Author: cupakromer

CHANGELOG.md

@@ -8,6 +8,8 @@ Bug Fixes:
 - Fix bug causing brief scan dropouts after starting a scan after a long period
   of inactivity (i.e. startup and background-foreground transitions) due to
   Android N scan limits (#489, Aaron Kromer)
+- Ensure thread safety for singleton creation of `BeaconManager`,
+  `DetectionTracker`, `MonitoringStatus`, and `Stats`. (#494, Aaron Kromer)
 
 
 ### 2.9.2 / 2016-11-22

src/main/java/org/altbeacon/beacon/BeaconManager.java

@@ -108,7 +108,7 @@
 public class BeaconManager {
     private static final String TAG = "BeaconManager";
     private Context mContext;
-    protected static BeaconManager client = null;
+    protected static volatile BeaconManager client = null;
     private final ConcurrentMap<BeaconConsumer, ConsumerInfo> consumers = new ConcurrentHashMap<BeaconConsumer,ConsumerInfo>();
     private Messenger serviceMessenger = null;
     protected final Set<RangeNotifier> rangeNotifiers = new CopyOnWriteArraySet<>();
@@ -123,6 +123,11 @@ public class BeaconManager {
     private static boolean sAndroidLScanningDisabled = false;
     private static boolean sManifestCheckingDisabled = false;
 
+    /**
+     * Private lock object for singleton initialization protecting against denial-of-service attack.
+     */
+    private static final Object SINGLETON_LOCK = new Object();
+
     /**
      * Set to true if you want to show library debugging.
      *
@@ -239,11 +244,29 @@ public static long getRegionExitPeriod(){
      * or non-Service class, you can attach it to another singleton or a subclass of the Android Application class.
      */
     public static BeaconManager getInstanceForApplication(Context context) {
-        if (client == null) {
-            LogManager.d(TAG, "BeaconManager instance creation");
-            client = new BeaconManager(context);
+        /*
+         * Follow double check pattern from Effective Java v2 Item 71.
+         *
+         * Bloch recommends using the local variable for this for performance reasons:
+         *
+         * > What this variable does is ensure that `field` is read only once in the common case
+         * > where it's already initialized. While not strictly necessary, this may improve
+         * > performance and is more elegant by the standards applied to low-level concurrent
+         * > programming. On my machine, [this] is about 25 percent faster than the obvious
+         * > version without a local variable.
+         *
+         * Joshua Bloch. Effective Java, Second Edition. Addison-Wesley, 2008. pages 283-284
+         */
+        BeaconManager instance = client;
+        if (instance == null) {
+            synchronized (SINGLETON_LOCK) {
+                instance = client;
+                if (instance == null) {
+                    client = instance = new BeaconManager(context);
+                }
+            }
         }
-        return client;
+        return instance;
     }
 
    protected BeaconManager(Context context) {

src/main/java/org/altbeacon/beacon/service/DetectionTracker.java

@@ -6,16 +6,14 @@
  * Created by dyoung on 1/10/15.
  */
 public class DetectionTracker {
-    private static DetectionTracker sDetectionTracker = null;
+    private static final DetectionTracker INSTANCE = new DetectionTracker();
+
     private long mLastDetectionTime = 0l;
     private DetectionTracker() {
 
     }
-    public static synchronized DetectionTracker getInstance() {
-        if (sDetectionTracker == null) {
-            sDetectionTracker  = new DetectionTracker();
-        }
-        return sDetectionTracker;
+    public static DetectionTracker getInstance() {
+        return INSTANCE;
     }
     public long getLastDetectionTime() {
         return mLastDetectionTime;

src/main/java/org/altbeacon/beacon/service/MonitoringStatus.java

@@ -23,7 +23,7 @@
 import static android.content.Context.MODE_PRIVATE;
 
 public class MonitoringStatus {
-    private static MonitoringStatus sInstance;
+    private static volatile MonitoringStatus sInstance;
     private static final int MAX_REGIONS_FOR_STATUS_PRESERVATION = 50;
     private static final int MAX_STATUS_PRESERVATION_FILE_AGE_TO_RESTORE_SECS = 60 * 15;
     private static final String TAG = MonitoringStatus.class.getSimpleName();
@@ -35,15 +35,35 @@ public class MonitoringStatus {
 
     private boolean mStatePreservationIsOn = true;
 
+    /**
+     * Private lock object for singleton initialization protecting against denial-of-service attack.
+     */
+    private static final Object SINGLETON_LOCK = new Object();
+
     public static MonitoringStatus getInstanceForApplication(Context context) {
-        if (sInstance == null) {
-            synchronized (MonitoringStatus.class) {
-                if (sInstance == null) {
-                    sInstance = new MonitoringStatus(context.getApplicationContext());
+        /*
+         * Follow double check pattern from Effective Java v2 Item 71.
+         *
+         * Bloch recommends using the local variable for this for performance reasons:
+         *
+         * > What this variable does is ensure that `field` is read only once in the common case
+         * > where it's already initialized. While not strictly necessary, this may improve
+         * > performance and is more elegant by the standards applied to low-level concurrent
+         * > programming. On my machine, [this] is about 25 percent faster than the obvious
+         * > version without a local variable.
+         *
+         * Joshua Bloch. Effective Java, Second Edition. Addison-Wesley, 2008. pages 283-284
+         */
+        MonitoringStatus instance = sInstance;
+        if (instance == null) {
+            synchronized (SINGLETON_LOCK) {
+                instance = sInstance;
+                if (instance == null) {
+                    sInstance = instance = new MonitoringStatus(context.getApplicationContext());
                 }
             }
         }
-        return sInstance;
+        return instance;
     }
 
     public MonitoringStatus(Context context) {

src/main/java/org/altbeacon/beacon/service/Stats.java

@@ -12,6 +12,7 @@
  * Created by dyoung on 10/16/14.
  */
 public class Stats {
+    private static final Stats INSTANCE = new Stats();
     private static final String TAG = "Stats";
     private static final SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("HH:mm:ss.SSS");
 
@@ -21,14 +22,11 @@ public class Stats {
     private boolean mEnableHistoricalLogging;
     private boolean mEnabled;
     private Sample mSample;
-    private static Stats mInstance;
 
     public static Stats getInstance() {
-        if(mInstance == null) {
-            mInstance = new Stats();
-        }
-        return mInstance;
+        return INSTANCE;
     }
+
     private Stats() {
         mSampleIntervalMillis = 0l;
         clearSamples();