From fbdffa70b01f5a7723b1529062da9653ec346566 Mon Sep 17 00:00:00 2001
From: Grantland Chew <grantlandchew@gmail.com>
Date: Tue, 20 Oct 2015 16:29:31 -0700
Subject: [PATCH] Fix: Multiple "current" ParseInstallation instances being
 created

This fixes a race condition where multiple `ParseInstallation`
instances can be returned as "current".
---
 .../CachedCurrentInstallationController.java  | 49 ++++++++++---------
 ...chedCurrentInstallationControllerTest.java | 29 +++++++++++
 2 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/Parse/src/main/java/com/parse/CachedCurrentInstallationController.java b/Parse/src/main/java/com/parse/CachedCurrentInstallationController.java
index 335145113..79f8938e5 100644
--- a/Parse/src/main/java/com/parse/CachedCurrentInstallationController.java
+++ b/Parse/src/main/java/com/parse/CachedCurrentInstallationController.java
@@ -70,13 +70,10 @@ public Task<Void> then(Task<Void> task) throws Exception {
 
   @Override
   public Task<ParseInstallation> getAsync() {
-    final ParseInstallation cachedCurrent;
     synchronized (mutex) {
-      cachedCurrent = currentInstallation;
-    }
-
-    if (cachedCurrent != null) {
-      return Task.forResult(cachedCurrent);
+      if (currentInstallation != null) {
+        return Task.forResult(currentInstallation);
+      }
     }
 
     return taskQueue.enqueue(new Continuation<Void, Task<ParseInstallation>>() {
@@ -85,26 +82,32 @@ public Task<ParseInstallation> then(Task<Void> toAwait) throws Exception {
         return toAwait.continueWithTask(new Continuation<Void, Task<ParseInstallation>>() {
           @Override
           public Task<ParseInstallation> then(Task<Void> task) throws Exception {
-            return store.getAsync();
-          }
-        }).continueWith(new Continuation<ParseInstallation, ParseInstallation>() {
-          @Override
-          public ParseInstallation then(Task<ParseInstallation> task) throws Exception {
-            ParseInstallation current = task.getResult();
-            if (current == null) {
-              current = ParseObject.create(ParseInstallation.class);
-              current.updateDeviceInfo(installationId);
-            } else {
-              installationId.set(current.getInstallationId());
-              PLog.v(TAG, "Successfully deserialized Installation object");
-            }
-
             synchronized (mutex) {
-              currentInstallation = current;
+              if (currentInstallation != null) {
+                return Task.forResult(currentInstallation);
+              }
             }
-            return current;
+
+            return store.getAsync().continueWith(new Continuation<ParseInstallation, ParseInstallation>() {
+              @Override
+              public ParseInstallation then(Task<ParseInstallation> task) throws Exception {
+                ParseInstallation current = task.getResult();
+                if (current == null) {
+                  current = ParseObject.create(ParseInstallation.class);
+                  current.updateDeviceInfo(installationId);
+                } else {
+                  installationId.set(current.getInstallationId());
+                  PLog.v(TAG, "Successfully deserialized Installation object");
+                }
+
+                synchronized (mutex) {
+                  currentInstallation = current;
+                }
+                return current;
+              }
+            }, ParseExecutors.io());
           }
-        }, ParseExecutors.io());
+        });
       }
     });
   }
diff --git a/Parse/src/test/java/com/parse/CachedCurrentInstallationControllerTest.java b/Parse/src/test/java/com/parse/CachedCurrentInstallationControllerTest.java
index db2af5aed..8bcf72bb8 100644
--- a/Parse/src/test/java/com/parse/CachedCurrentInstallationControllerTest.java
+++ b/Parse/src/test/java/com/parse/CachedCurrentInstallationControllerTest.java
@@ -144,6 +144,35 @@ public void testGetAsyncWithNoInstallation() throws Exception {
     assertEquals("android", currentInstallation.get(KEY_DEVICE_TYPE));
   }
 
+  @Test
+  public void testGetAsyncWithNoInstallationRaceCondition() throws ParseException {
+    // Mock installationId
+    InstallationId installationId = mock(InstallationId.class);
+    when(installationId.get()).thenReturn("testInstallationId");
+    //noinspection unchecked
+    ParseObjectStore<ParseInstallation> store = mock(ParseObjectStore.class);
+    Task<ParseInstallation>.TaskCompletionSource tcs = Task.create();
+    when(store.getAsync()).thenReturn(tcs.getTask());
+
+    // Create test controller
+    CachedCurrentInstallationController controller =
+        new CachedCurrentInstallationController(store, installationId);
+
+    Task<ParseInstallation> taskA = controller.getAsync();
+    Task<ParseInstallation> taskB = controller.getAsync();
+
+    tcs.setResult(null);
+    ParseInstallation installationA = ParseTaskUtils.wait(taskA);
+    ParseInstallation installationB = ParseTaskUtils.wait(taskB);
+
+    verify(store, times(1)).getAsync();
+    assertSame(controller.currentInstallation, installationA);
+    assertSame(controller.currentInstallation, installationB);
+    // Make sure device info is updated
+    assertEquals("testInstallationId", installationA.getInstallationId());
+    assertEquals("android", installationA.get(KEY_DEVICE_TYPE));
+  }
+
   //endregion
 
   //region testExistsAsync