Bug 1255657 - Write client ID file before Gecko starts to prevent race. r=rnewman
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 06 Apr 2016 15:07:01 -0700
changeset 316856 747517785de246ccef4e29b8c7d2f0ee75c9624d
parent 316855 fdec59273728a0c962f8d93d1b6e84d3aa2acc1e
child 316857 927d52d615857dbc5f8e085e288a10b3fafeab60
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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
@@ -620,46 +620,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 {
@@ -978,16 +985,17 @@ public final class GeckoProfile {
 
     private File findProfileDir() throws NoSuchProfileException {
         if (isCustomProfile()) {
             return mProfileDir;
         }
         return GeckoProfileDirectories.findProfileDir(mMozillaDir, mName);
     }
 
+    @WorkerThread
     private File createProfileDir() throws IOException {
         if (isCustomProfile()) {
             // Custom profiles must already exist.
             return mProfileDir;
         }
 
         INIParser parser = GeckoProfileDirectories.getProfilesINI(mMozillaDir);
 
@@ -1053,16 +1061,21 @@ public final class GeckoProfile {
             } finally {
                 writer.close();
             }
         } catch (Exception e) {
             // Best-effort.
             Log.w(LOGTAG, "Couldn't write " + TIMES_PATH, 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;
     }
 
     /**