Bug 1255657 - Write client ID file before Gecko starts to prevent race. r=rnewman draft
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 06 Apr 2016 15:07:01 -0700
changeset 348190 7d1ff8a354accfaae0dc3f2d222ed32f6aa8cbf6
parent 347397 20df8310f734ecc938f1ad53e27759253ae14040
child 517804 3a68fde30cabf727fe919d646308921d947976f9
push id14775
push usermichael.l.comella@gmail.com
push dateWed, 06 Apr 2016 22:09:05 +0000
reviewersrnewman
bugs1255657, 1262625
milestone48.0a1
Bug 1255657 - Write client ID file before Gecko starts to prevent race. r=rnewman As best as I can tell, this code runs before Gecko is initialized. via bug 1255657 comment 8: To confirm this, GeckoProfile is created before the GeckoThread is finished launching but it seems almost coincidental: GeckoApp.onCreate GeckoThread.launch GeckoThread.start ... GeckoThread.run GeckoThread.getGeckoArgs (notably before GeckoLader.nativeRun) GeckoThread.addCustomProfileArg (if GAP.getGeckoInterface != null, which is set earlier in GeckoApp.onCreate) EITHER GeckoProfile.getDir() (if in guest mode) GeckoProfile.forceCreate GeckoProfile.forceCreate (if not in guest mode) forceCreate opens the times.json file. That being said, if this code path changes, forceCreate is also called when the GeckoView is initialized (which occurs after the GeckoThread.launch call, but is likely to happen before Gecko finishes launching). --- If we wanted GeckoProfile initialization timing to be seem less coincidental, we should consider initializing the profile non-lazily: bug 1262625. MozReview-Commit-ID: LGluC021CTg
mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
@@ -626,46 +626,53 @@ public final class GeckoProfile {
         }
 
         String clientIdToWrite;
         try {
             clientIdToWrite = getValidClientIdFromDisk(FHR_CLIENT_ID_FILE_PATH);
         } catch (final IOException e) {
             // Avoid log spam: don't log the full Exception w/ the stack trace.
             Log.d(LOGTAG, "Could not migrate client ID from FHR – creating a new one: " + e.getLocalizedMessage());
-            clientIdToWrite = UUID.randomUUID().toString();
+            clientIdToWrite = generateNewClientId();
         }
 
         // There is a possibility Gecko is running and the Gecko telemetry implementation decided it's time to generate
         // the client ID, writing client ID underneath us. Since it's highly unlikely (e.g. we run in onStart before
         // Gecko is started), we don't handle that possibility besides writing the ID and then reading from the file
         // again (rather than just returning the value we generated before writing).
         //
         // In the event it does happen, any discrepancy will be resolved after a restart. In the mean time, both this
         // implementation and the Gecko implementation could upload documents with inconsistent IDs.
         //
         // In any case, if we get an exception, intentionally throw - there's nothing more to do here.
         persistClientId(clientIdToWrite);
         return getValidClientIdFromDisk(CLIENT_ID_FILE_PATH);
     }
 
+    protected static String generateNewClientId() {
+        return UUID.randomUUID().toString();
+    }
+
     /**
      * @return a valid client ID
      * @throws IOException if a valid client ID could not be retrieved
      */
     @WorkerThread
     private String getValidClientIdFromDisk(final String filePath) throws IOException {
         final JSONObject obj = readJSONObjectFromFile(filePath);
         final String clientId = obj.optString(CLIENT_ID_JSON_ATTR);
         if (isClientIdValid(clientId)) {
             return clientId;
         }
         throw new IOException("Received client ID is invalid: " + clientId);
     }
 
+    /**
+     * Persists the given client ID to disk. This will overwrite any existing files.
+     */
     @WorkerThread
     private void persistClientId(final String clientId) throws IOException {
         if (!ensureParentDirs(CLIENT_ID_FILE_PATH)) {
             throw new IOException("Could not create client ID parent directories");
         }
 
         final JSONObject obj = new JSONObject();
         try {
@@ -955,16 +962,17 @@ public final class GeckoProfile {
         sDefaultProfileName = profileName;
         return sDefaultProfileName;
     }
 
     private File findProfileDir() throws NoSuchProfileException {
         return GeckoProfileDirectories.findProfileDir(mMozillaDir, mName);
     }
 
+    @WorkerThread
     private File createProfileDir() throws IOException {
         INIParser parser = GeckoProfileDirectories.getProfilesINI(mMozillaDir);
 
         // Salt the name of our requested profile
         String saltedName = GeckoProfileDirectories.saltProfileName(mName);
         File profileDir = new File(mMozillaDir, saltedName);
         while (profileDir.exists()) {
             saltedName = GeckoProfileDirectories.saltProfileName(mName);
@@ -1025,16 +1033,21 @@ public final class GeckoProfile {
             } finally {
                 writer.close();
             }
         } catch (Exception e) {
             // Best-effort.
             Log.w(LOGTAG, "Couldn't write times.json.", e);
         }
 
+        // Create the client ID file before Gecko starts (we assume this method
+        // is called before Gecko starts). If we let Gecko start, the JS telemetry
+        // code may try to write to the file at the same time Java does.
+        persistClientId(generateNewClientId());
+
         // Initialize pref flag for displaying the start pane for a new profile.
         final SharedPreferences prefs = GeckoSharedPrefs.forProfile(mApplicationContext);
         prefs.edit().putBoolean(FirstrunAnimationContainer.PREF_FIRSTRUN_ENABLED, true).apply();
 
         return profileDir;
     }
 
     /**