Bug 1255657 - Write client ID file before Gecko starts to prevent race. r=rnewman, a=ritu
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 06 Apr 2016 15:07:01 -0700
changeset 325874 02b8dc35ec80a95dd4f1cdd5a4075718aed233f2
parent 325873 d6f797000c07b0d328b8843524de953e678c7379
child 325875 133c7279577b12d9814cbebdd09d529f7c75a0f6
push id1128
push userjlund@mozilla.com
push dateWed, 01 Jun 2016 01:31:59 +0000
treeherdermozilla-release@fe0d30de989d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, ritu
bugs1255657, 1262625
milestone47.0a2
Bug 1255657 - Write client ID file before Gecko starts to prevent race. r=rnewman, a=ritu 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;
     }
 
     /**