From cad2f559e3778e26fcd82174f61cfd381ec8d48d Mon Sep 17 00:00:00 2001
From: Sergey Nikolaienkov <sergeynv@google.com>
Date: Fri, 14 Jan 2022 14:01:21 +0100
Subject: [PATCH] Explain synchronization in CDM's PersistentDataStore

Add comments to PersistentDataStore.java that explain how we synchronize
accesses to the back-up files in that class.

Test: N/A
Change-Id: I1246abd1c3752c1eebb1952989e07c30c06283ed
---
 .../server/companion/PersistentDataStore.java | 60 ++++++++++++-------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/services/companion/java/com/android/server/companion/PersistentDataStore.java b/services/companion/java/com/android/server/companion/PersistentDataStore.java
index ef3aa7fea1b54..3c8c3cb8f8420 100644
--- a/services/companion/java/com/android/server/companion/PersistentDataStore.java
+++ b/services/companion/java/com/android/server/companion/PersistentDataStore.java
@@ -67,17 +67,20 @@ import java.util.concurrent.ConcurrentMap;
  * The class responsible for persisting Association records and other related information (such as
  * previously used IDs) to a disk, and reading the data back from the disk.
  *
- * Before Android T the data was stored to `companion_device_manager_associations.xml` file in
- * {@link Environment#getUserSystemDirectory(int)}
- * (eg. `/data/system/users/0/companion_device_manager_associations.xml`)
- * @see #getBaseLegacyStorageFileForUser(int)
+ * <p>
+ * Before Android T the data was stored in "companion_device_manager_associations.xml" file in
+ * {@link Environment#getUserSystemDirectory(int) /data/system/user/}.
  *
- * Before Android T the data was stored using the v0 schema.
+ * See {@link #getBaseLegacyStorageFileForUser(int) getBaseLegacyStorageFileForUser()}.
  *
- * @see #readAssociationsV0(TypedXmlPullParser, int, Collection)
- * @see #readAssociationV0(TypedXmlPullParser, int, int, Collection)
+ * <p>
+ * Before Android T the data was stored using the v0 schema. See:
+ * <ul>
+ * <li>{@link #readAssociationsV0(TypedXmlPullParser, int, Collection) readAssociationsV0()}.
+ * <li>{@link #readAssociationV0(TypedXmlPullParser, int, int, Collection) readAssociationV0()}.
+ * </ul>
  *
- * The following snippet is a sample of a the file that is using v0 schema.
+ * The following snippet is a sample of a file that is using v0 schema.
  * <pre>{@code
  * <associations>
  *   <association
@@ -93,24 +96,28 @@ import java.util.concurrent.ConcurrentMap;
  * </associations>
  * }</pre>
  *
+ * <p>
+ * Since Android T the data is stored to "companion_device_manager.xml" file in
+ * {@link Environment#getDataSystemDeDirectory(int) /data/system_de/}.
  *
- * Since Android T the data is stored to `companion_device_manager.xml` file in
- * {@link Environment#getDataSystemDeDirectory(int)}.
- * (eg. `/data/system_de/0/companion_device_manager.xml`)
- * @see #getBaseStorageFileForUser(int)
-
+ * See {@link #getBaseStorageFileForUser(int) getBaseStorageFileForUser()}
+ *
+ * <p>
  * Since Android T the data is stored using the v1 schema.
- * In the v1 schema, a list of the previously used IDs is storead along with the association
+ *
+ * In the v1 schema, a list of the previously used IDs is stored along with the association
  * records.
- * V1 schema adds a new optional `display_name` attribute, and makes the `mac_address` attribute
+ *
+ * V1 schema adds a new optional "display_name" attribute, and makes the "mac_address" attribute
  * optional.
+ * <ul>
+ * <li> {@link #CURRENT_PERSISTENCE_VERSION}
+ * <li> {@link #readAssociationsV1(TypedXmlPullParser, int, Collection) readAssociationsV1()}
+ * <li> {@link #readAssociationV1(TypedXmlPullParser, int, Collection) readAssociationV1()}
+ * <li> {@link #readPreviouslyUsedIdsV1(TypedXmlPullParser, Map) readPreviouslyUsedIdsV1()}
+ * </ul>
  *
- * @see #CURRENT_PERSISTENCE_VERSION
- * @see #readAssociationsV1(TypedXmlPullParser, int, Collection)
- * @see #readAssociationV1(TypedXmlPullParser, int, Collection)
- * @see #readPreviouslyUsedIdsV1(TypedXmlPullParser, Map)
- *
- * The following snippet is a sample of a the file that is using v0 schema.
+ * The following snippet is a sample of a file that is using v0 schema.
  * <pre>{@code
  * <state persistence-version="1">
  *     <associations>
@@ -206,6 +213,8 @@ final class PersistentDataStore {
         final AtomicFile file = getStorageFileForUser(userId);
         if (DEBUG) Slog.d(LOG_TAG, "  > File=" + file.getBaseFile().getPath());
 
+        // getStorageFileForUser() ALWAYS returns the SAME OBJECT, which allows us to synchronize
+        // accesses to the file on the file system using this AtomicFile object.
         synchronized (file) {
             File legacyBaseFile = null;
             final AtomicFile readFrom;
@@ -269,6 +278,8 @@ final class PersistentDataStore {
 
         final AtomicFile file = getStorageFileForUser(userId);
         if (DEBUG) Slog.d(LOG_TAG, "  > File=" + file.getBaseFile().getPath());
+        // getStorageFileForUser() ALWAYS returns the SAME OBJECT, which allows us to synchronize
+        // accesses to the file on the file system using this AtomicFile object.
         synchronized (file) {
             persistStateToFileLocked(file, associations, previouslyUsedIdsPerPackage);
         }
@@ -329,6 +340,13 @@ final class PersistentDataStore {
         });
     }
 
+    /**
+     * Creates and caches {@link AtomicFile} object that represents the back-up file for the given
+     * user.
+     *
+     * IMPORTANT: the method will ALWAYS return the same {@link AtomicFile} object, which makes it
+     * possible to synchronize reads and writes to the file using the returned object.
+     */
     private @NonNull AtomicFile getStorageFileForUser(@UserIdInt int userId) {
         return mUserIdToStorageFile.computeIfAbsent(userId,
                 u -> new AtomicFile(getBaseStorageFileForUser(userId)));