Skip to content

Commit 1a36dd7

Browse files
authored
Completing getId call with the disk reads on the caller thread. (#1570)
* Dropping guava and appcompat dependency from FIS SDK. * Fix OverlappingFileLockException. Access the data shared by multiple threads only after acquiring cross-process and multi-thread safe locks. * Completing getId call with the disk reads on the caller thread. Also, fixing delete API to access persisted FID using cross process safe locks. * Addressing Fred's comments * Remove unused variable * Cache FID to avoid multiple disk reads. (#1571) * Cache FID to avoid multiple disk reads. * Addressing Rayo's comments. * Addressing Rayo's comments
1 parent b1172c9 commit 1a36dd7

File tree

2 files changed

+56
-76
lines changed

2 files changed

+56
-76
lines changed

firebase-installations/src/main/java/com/google/firebase/installations/FirebaseInstallations.java

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,16 @@ public class FirebaseInstallations implements FirebaseInstallationsApi {
6767
private final Object lock = new Object();
6868
private final ExecutorService backgroundExecutor;
6969
private final ExecutorService networkExecutor;
70+
/* FID of this Firebase Installations instance. Cached after successfully registering and
71+
persisting the FID locally. NOTE: cachedFid resets if FID is deleted.*/
72+
private String cachedFid = null;
7073

7174
@GuardedBy("lock")
7275
private final List<StateListener> listeners = new ArrayList<>();
7376

7477
/* used for thread-level synchronization of generating and persisting fids */
7578
private static final Object lockGenerateFid = new Object();
79+
7680
/* file used for process-level synchronization of generating and persisting fids */
7781
private static final String LOCKFILE_NAME_GENERATE_FID = "generatefid.lock";
7882
private static final String CHIME_FIREBASE_APP_NAME = "CHIME_ANDROID_SDK";
@@ -213,9 +217,9 @@ String getName() {
213217
@Override
214218
public Task<String> getId() {
215219
preConditionChecks();
216-
Task<String> task = addGetIdListener();
217-
backgroundExecutor.execute(this::doGetId);
218-
return task;
220+
TaskCompletionSource<String> taskCompletionSource = new TaskCompletionSource<>();
221+
taskCompletionSource.trySetResult(doGetId());
222+
return taskCompletionSource.getTask();
219223
}
220224

221225
/**
@@ -231,11 +235,7 @@ public Task<String> getId() {
231235
public Task<InstallationTokenResult> getToken(boolean forceRefresh) {
232236
preConditionChecks();
233237
Task<InstallationTokenResult> task = addGetAuthTokenListener();
234-
if (forceRefresh) {
235-
backgroundExecutor.execute(this::doGetAuthTokenForceRefresh);
236-
} else {
237-
backgroundExecutor.execute(this::doGetAuthTokenWithoutForceRefresh);
238-
}
238+
backgroundExecutor.execute(() -> doGetAuthToken(forceRefresh));
239239
return task;
240240
}
241241

@@ -250,15 +250,6 @@ public Task<Void> delete() {
250250
return Tasks.call(backgroundExecutor, this::deleteFirebaseInstallationId);
251251
}
252252

253-
private Task<String> addGetIdListener() {
254-
TaskCompletionSource<String> taskCompletionSource = new TaskCompletionSource<>();
255-
StateListener l = new GetIdListener(taskCompletionSource);
256-
synchronized (lock) {
257-
listeners.add(l);
258-
}
259-
return taskCompletionSource.getTask();
260-
}
261-
262253
private Task<InstallationTokenResult> addGetAuthTokenListener() {
263254
TaskCompletionSource<InstallationTokenResult> taskCompletionSource =
264255
new TaskCompletionSource<>();
@@ -295,16 +286,15 @@ private void triggerOnException(PersistedInstallationEntry prefs, Exception exce
295286
}
296287
}
297288

298-
private final void doGetId() {
299-
doRegistrationInternal(false);
300-
}
301-
302-
private final void doGetAuthTokenWithoutForceRefresh() {
303-
doRegistrationInternal(false);
304-
}
305-
306-
private final void doGetAuthTokenForceRefresh() {
307-
doRegistrationInternal(true);
289+
private String doGetId() {
290+
if (cachedFid != null) {
291+
return cachedFid;
292+
}
293+
PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe();
294+
// Execute network calls (CreateInstallations) to the FIS Servers on a separate executor
295+
// i.e networkExecutor
296+
networkExecutor.execute(() -> doNetworkCallIfNecessary(false));
297+
return prefs.getFirebaseInstallationId();
308298
}
309299

310300
/**
@@ -316,7 +306,7 @@ private final void doGetAuthTokenForceRefresh() {
316306
* @param forceRefresh true if this is for a getAuthToken call and if the caller wants to fetch a
317307
* new auth token from the server even if an unexpired auth token exists on the client.
318308
*/
319-
private final void doRegistrationInternal(boolean forceRefresh) {
309+
private void doGetAuthToken(boolean forceRefresh) {
320310
PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe();
321311

322312
// Since the caller wants to force an authtoken refresh remove the authtoken from the
@@ -328,11 +318,11 @@ private final void doRegistrationInternal(boolean forceRefresh) {
328318
triggerOnStateReached(prefs);
329319
// Execute network calls (CreateInstallations or GenerateAuthToken) to the FIS Servers on
330320
// a separate executor i.e networkExecutor
331-
networkExecutor.execute(() -> doNetworkCall(forceRefresh));
321+
networkExecutor.execute(() -> doNetworkCallIfNecessary(forceRefresh));
332322
}
333323

334-
private void doNetworkCall(boolean forceRefresh) {
335-
PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe();
324+
private void doNetworkCallIfNecessary(boolean forceRefresh) {
325+
PersistedInstallationEntry prefs = getMultiProcessSafePrefs();
336326
// There are two possible cleanup steps to perform at this stage: the FID may need to
337327
// be registered with the server or the FID is registered but we need a fresh authtoken.
338328
// Registering will also result in a fresh authtoken. Do the appropriate step here.
@@ -353,6 +343,11 @@ private void doNetworkCall(boolean forceRefresh) {
353343
// Store the prefs to persist the result of the previous step.
354344
insertOrUpdatePrefs(prefs);
355345

346+
// Update cachedFID, if FID is successfully REGISTERED and persisted.
347+
if (prefs.isRegistered()) {
348+
cachedFid = prefs.getFirebaseInstallationId();
349+
}
350+
356351
// Let the caller know about the result.
357352
if (prefs.isErrored()) {
358353
triggerOnException(prefs, new FirebaseInstallationsException(Status.BAD_CONFIG));
@@ -509,6 +504,7 @@ private PersistedInstallationEntry fetchAuthTokenFromServer(
509504
case AUTH_ERROR:
510505
// The the server refused to generate a new auth token due to bad credentials, clear the
511506
// FID to force the generation of a new one.
507+
cachedFid = null;
512508
return prefs.withNoGeneratedFid();
513509
default:
514510
throw new IOException();
@@ -520,7 +516,8 @@ private PersistedInstallationEntry fetchAuthTokenFromServer(
520516
* storage.
521517
*/
522518
private Void deleteFirebaseInstallationId() throws FirebaseInstallationsException, IOException {
523-
PersistedInstallationEntry entry = persistedInstallation.readPersistedInstallationEntryValue();
519+
cachedFid = null;
520+
PersistedInstallationEntry entry = getMultiProcessSafePrefs();
524521
if (entry.isRegistered()) {
525522
// Call the FIS servers to delete this Firebase Installation Id.
526523
try {
@@ -535,8 +532,34 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio
535532
"Failed to delete a Firebase Installation.", Status.BAD_CONFIG);
536533
}
537534
}
538-
539535
insertOrUpdatePrefs(entry.withNoGeneratedFid());
540536
return null;
541537
}
538+
539+
/**
540+
* Loads the persisted prefs. This operation is made cross-process and cross-thread safe by
541+
* wrapping all the processing first in a java synchronization block and wrapping that in a
542+
* cross-process lock created using FileLocks.
543+
*
544+
* @return a persisted prefs
545+
*/
546+
private PersistedInstallationEntry getMultiProcessSafePrefs() {
547+
synchronized (lockGenerateFid) {
548+
CrossProcessLock lock =
549+
CrossProcessLock.acquire(firebaseApp.getApplicationContext(), LOCKFILE_NAME_GENERATE_FID);
550+
try {
551+
PersistedInstallationEntry prefs =
552+
persistedInstallation.readPersistedInstallationEntryValue();
553+
return prefs;
554+
555+
} finally {
556+
// It is possible that the lock acquisition failed, resulting in lock being null.
557+
// We handle this case by going on with our business even if the acquisition failed
558+
// but we need to be sure to only release if we got a lock.
559+
if (lock != null) {
560+
lock.releaseAndClose();
561+
}
562+
}
563+
}
564+
}
542565
}

firebase-installations/src/main/java/com/google/firebase/installations/GetIdListener.java

Lines changed: 0 additions & 43 deletions
This file was deleted.

0 commit comments

Comments
 (0)