Skip to content

Commit 221431a

Browse files
committed
Fix deadlock in AccMgrAuthTokenProvider when account is removed during token refresh
When logout occurs while threads are refreshing tokens, the method will return for matchingAccount == null and therefore the lock.notifyAll() that is later in the method will never be called causing other threads to be stuck on lock.wait(). Now the try-finally wraps all the logic after gettingAuthToken = true to ensure that lock.notifyAll() is always called. Fixes W-21304558
1 parent 6c1cbf3 commit 221431a

1 file changed

Lines changed: 27 additions & 20 deletions

File tree

libs/SalesforceSDK/src/com/salesforce/androidsdk/rest/ClientManager.java

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -393,30 +393,35 @@ public String getNewAuthToken() {
393393
gettingAuthToken = true;
394394
}
395395

396-
// Only check for matching account inside synchronized thread that
397-
// is actually getting the new auth token.
398-
UserAccountManager userAccountManager = SalesforceSDKManager.getInstance().getUserAccountManager();
399-
Account[] accounts = clientManager.getAccounts();
400-
Account matchingAccount = null;
401396
String newAuthToken = null;
402397
String newInstanceUrl = null;
398+
boolean shouldUpdateCache = false;
403399

404-
if (refreshToken != null) {
405-
for (Account account : accounts) {
406-
UserAccount user = userAccountManager.buildUserAccount(account);
407-
if (user != null && refreshToken.equals(user.getRefreshToken())) {
408-
matchingAccount = account;
409-
break;
400+
try {
401+
// Only check for matching account inside synchronized thread that
402+
// is actually getting the new auth token.
403+
UserAccountManager userAccountManager = SalesforceSDKManager.getInstance().getUserAccountManager();
404+
Account[] accounts = clientManager.getAccounts();
405+
Account matchingAccount = null;
406+
407+
if (refreshToken != null) {
408+
for (Account account : accounts) {
409+
UserAccount user = userAccountManager.buildUserAccount(account);
410+
if (user != null && refreshToken.equals(user.getRefreshToken())) {
411+
matchingAccount = account;
412+
break;
413+
}
410414
}
411415
}
412-
}
413416

414-
// Fail early to ensure we don't logout the current user below by sending null.
415-
if (matchingAccount == null) {
416-
return null;
417-
}
417+
// Fail early to ensure we don't logout the current user below by sending null.
418+
if (matchingAccount == null) {
419+
return null;
420+
}
421+
422+
// We found a matching account, so we'll attempt a refresh and should update the cache.
423+
shouldUpdateCache = true;
418424

419-
try {
420425
// Invalidate current auth token.
421426
clientManager.invalidateToken(lastNewAuthToken);
422427
final UserAccount userAccount = refreshStaleToken(matchingAccount);
@@ -460,9 +465,11 @@ public String getNewAuthToken() {
460465
} finally {
461466
synchronized (lock) {
462467
gettingAuthToken = false;
463-
lastNewAuthToken = newAuthToken;
464-
lastNewInstanceUrl = newInstanceUrl;
465-
lastRefreshTime = System.currentTimeMillis();
468+
if (shouldUpdateCache) {
469+
lastNewAuthToken = newAuthToken;
470+
lastNewInstanceUrl = newInstanceUrl;
471+
lastRefreshTime = System.currentTimeMillis();
472+
}
466473
lock.notifyAll();
467474
}
468475
}

0 commit comments

Comments
 (0)