Skip to content

Completing getId call with the disk reads on the caller thread. #1570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
May 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
4283a2b
Dropping guava and appcompat dependency from FIS SDK.
ankitaj224 Feb 6, 2020
984884c
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Feb 6, 2020
a823877
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Feb 11, 2020
afcb14c
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Feb 13, 2020
5574e30
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 5, 2020
a5dd4dc
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 9, 2020
45312e6
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 10, 2020
f0be4fa
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 10, 2020
ac0397b
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 11, 2020
8f6b46e
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 12, 2020
686a998
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 16, 2020
3bf7c24
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 18, 2020
07ff7dd
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 30, 2020
f982f2d
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Mar 31, 2020
672526a
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Apr 5, 2020
1adf4a2
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Apr 7, 2020
4435ed0
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Apr 10, 2020
99f1f82
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 Apr 13, 2020
08d91f8
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 May 1, 2020
747598f
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 May 1, 2020
28cc955
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 May 5, 2020
519ae71
Fix OverlappingFileLockException.
ankitaj224 May 8, 2020
98e0e25
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 May 8, 2020
cba321b
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 May 8, 2020
28598b0
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 May 12, 2020
9519e70
Merge branch 'master' of github.com:firebase/firebase-android-sdk
ankitaj224 May 18, 2020
f0ff299
Completing getId call with the disk reads on the caller thread.
ankitaj224 May 18, 2020
bc6ec5c
Addressing Fred's comments
ankitaj224 May 18, 2020
b26d198
Remove unused variable
ankitaj224 May 18, 2020
0bb76d3
Cache FID to avoid multiple disk reads. (#1571)
ankitaj224 May 20, 2020
9214f9e
Merge branch 'master' of github.com:firebase/firebase-android-sdk int…
ankitaj224 May 20, 2020
a5f6558
Addressing Rayo's comments
ankitaj224 May 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,16 @@ public class FirebaseInstallations implements FirebaseInstallationsApi {
private final Object lock = new Object();
private final ExecutorService backgroundExecutor;
private final ExecutorService networkExecutor;
/* FID of this Firebase Installations instance. Cached after successfully registering and
persisting the FID locally. NOTE: cachedFid resets if FID is deleted.*/
private String cachedFid = null;

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

/* used for thread-level synchronization of generating and persisting fids */
private static final Object lockGenerateFid = new Object();

/* file used for process-level synchronization of generating and persisting fids */
private static final String LOCKFILE_NAME_GENERATE_FID = "generatefid.lock";
private static final String CHIME_FIREBASE_APP_NAME = "CHIME_ANDROID_SDK";
Expand Down Expand Up @@ -213,9 +217,9 @@ String getName() {
@Override
public Task<String> getId() {
preConditionChecks();
Task<String> task = addGetIdListener();
backgroundExecutor.execute(this::doGetId);
return task;
TaskCompletionSource<String> taskCompletionSource = new TaskCompletionSource<>();
taskCompletionSource.trySetResult(doGetId());
return taskCompletionSource.getTask();
}

/**
Expand All @@ -231,11 +235,7 @@ public Task<String> getId() {
public Task<InstallationTokenResult> getToken(boolean forceRefresh) {
preConditionChecks();
Task<InstallationTokenResult> task = addGetAuthTokenListener();
if (forceRefresh) {
backgroundExecutor.execute(this::doGetAuthTokenForceRefresh);
} else {
backgroundExecutor.execute(this::doGetAuthTokenWithoutForceRefresh);
}
backgroundExecutor.execute(() -> doGetAuthToken(forceRefresh));
return task;
}

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

private Task<String> addGetIdListener() {
TaskCompletionSource<String> taskCompletionSource = new TaskCompletionSource<>();
StateListener l = new GetIdListener(taskCompletionSource);
synchronized (lock) {
listeners.add(l);
}
return taskCompletionSource.getTask();
}

private Task<InstallationTokenResult> addGetAuthTokenListener() {
TaskCompletionSource<InstallationTokenResult> taskCompletionSource =
new TaskCompletionSource<>();
Expand Down Expand Up @@ -295,16 +286,15 @@ private void triggerOnException(PersistedInstallationEntry prefs, Exception exce
}
}

private final void doGetId() {
doRegistrationInternal(false);
}

private final void doGetAuthTokenWithoutForceRefresh() {
doRegistrationInternal(false);
}

private final void doGetAuthTokenForceRefresh() {
doRegistrationInternal(true);
private String doGetId() {
if (cachedFid != null) {
return cachedFid;
}
PersistedInstallationEntry prefs = getPrefsWithGeneratedIdMultiProcessSafe();
// Execute network calls (CreateInstallations) to the FIS Servers on a separate executor
// i.e networkExecutor
networkExecutor.execute(() -> doNetworkCallIfNecessary(false));
return prefs.getFirebaseInstallationId();
}

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

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

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

// Update cachedFID, if FID is successfully REGISTERED and persisted.
if (prefs.isRegistered()) {
cachedFid = prefs.getFirebaseInstallationId();
}

// Let the caller know about the result.
if (prefs.isErrored()) {
triggerOnException(prefs, new FirebaseInstallationsException(Status.BAD_CONFIG));
Expand Down Expand Up @@ -509,6 +504,7 @@ private PersistedInstallationEntry fetchAuthTokenFromServer(
case AUTH_ERROR:
// The the server refused to generate a new auth token due to bad credentials, clear the
// FID to force the generation of a new one.
cachedFid = null;
return prefs.withNoGeneratedFid();
default:
throw new IOException();
Expand All @@ -520,7 +516,8 @@ private PersistedInstallationEntry fetchAuthTokenFromServer(
* storage.
*/
private Void deleteFirebaseInstallationId() throws FirebaseInstallationsException, IOException {
PersistedInstallationEntry entry = persistedInstallation.readPersistedInstallationEntryValue();
cachedFid = null;
PersistedInstallationEntry entry = getMultiProcessSafePrefs();
if (entry.isRegistered()) {
// Call the FIS servers to delete this Firebase Installation Id.
try {
Expand All @@ -535,8 +532,34 @@ private Void deleteFirebaseInstallationId() throws FirebaseInstallationsExceptio
"Failed to delete a Firebase Installation.", Status.BAD_CONFIG);
}
}

insertOrUpdatePrefs(entry.withNoGeneratedFid());
return null;
}

/**
* Loads the persisted prefs. This operation is made cross-process and cross-thread safe by
* wrapping all the processing first in a java synchronization block and wrapping that in a
* cross-process lock created using FileLocks.
*
* @return a persisted prefs
*/
private PersistedInstallationEntry getMultiProcessSafePrefs() {
synchronized (lockGenerateFid) {
CrossProcessLock lock =
CrossProcessLock.acquire(firebaseApp.getApplicationContext(), LOCKFILE_NAME_GENERATE_FID);
try {
PersistedInstallationEntry prefs =
persistedInstallation.readPersistedInstallationEntryValue();
return prefs;

} finally {
// It is possible that the lock acquisition failed, resulting in lock being null.
// We handle this case by going on with our business even if the acquisition failed
// but we need to be sure to only release if we got a lock.
if (lock != null) {
lock.releaseAndClose();
}
}
}
}
}

This file was deleted.