fix(auth): resolve flakiness in ClientSideCredentialAccessBoundaryFactoryTest

Synchronize the decision to refresh and task creation to avoid race conditions.

Resolves: #12871

Author: lqiu96

google-auth-library-java/cab-token-generator/java/com/google/auth/credentialaccessboundary/ClientSideCredentialAccessBoundaryFactory.java

@@ -227,15 +227,26 @@ public AccessToken generateToken(CredentialAccessBoundary accessBoundary)
    */
   @VisibleForTesting
   void refreshCredentialsIfRequired() throws IOException {
-    RefreshType refreshType = determineRefreshType();
+    RefreshType refreshType;
+    RefreshTask currentRefreshTask;
+
+    // Synchronize both the decision to refresh and the task creation to prevent a race
+    // condition. Without this, multiple threads might concurrently determine a refresh is
+    // needed (e.g., in ASYNC mode) and return ASYNC. The first thread would start the refresh
+    // task. If that task completes quickly and clears the `refreshTask` reference, a subsequent
+    // thread that also determined a refresh was needed would then see `refreshTask` as null
+    // and incorrectly start a second, redundant refresh task.
+    synchronized (refreshLock) {
+      refreshType = determineRefreshType();
 
-    if (refreshType == RefreshType.NONE) {
-      // No refresh needed, token is still valid.
-      return;
-    }
+      if (refreshType == RefreshType.NONE) {
+        // No refresh needed, token is still valid.
+        return;
+      }
 
-    // If a refresh is required, create or retrieve the refresh task.
-    RefreshTask currentRefreshTask = getOrCreateRefreshTask();
+      // If a refresh is required, create or retrieve the refresh task.
+      currentRefreshTask = getOrCreateRefreshTask();
+    }
 
     // Handle the refresh based on the determined refresh type.
     switch (refreshType) {
@@ -283,16 +294,14 @@ void refreshCredentialsIfRequired() throws IOException {
     }
   }
 
+  // Assumes the caller holds refreshLock.
   private RefreshType determineRefreshType() {
-    AccessToken intermediateAccessToken;
-    synchronized (refreshLock) {
-      if (intermediateCredentials == null
-          || intermediateCredentials.intermediateAccessToken == null) {
-        // A blocking refresh is needed if the intermediate access token doesn't exist.
-        return RefreshType.BLOCKING;
-      }
-      intermediateAccessToken = intermediateCredentials.intermediateAccessToken;
+    if (intermediateCredentials == null
+        || intermediateCredentials.intermediateAccessToken == null) {
+      // A blocking refresh is needed if the intermediate access token doesn't exist.
+      return RefreshType.BLOCKING;
     }
+    AccessToken intermediateAccessToken = intermediateCredentials.intermediateAccessToken;
 
     Date expirationTime = intermediateAccessToken.getExpirationTime();
     if (expirationTime == null) {
@@ -322,23 +331,22 @@ private RefreshType determineRefreshType() {
    * responsibility of the caller to execute it. The task will clear the single flight slot upon
    * completion.
    */
+  // Assumes the caller holds refreshLock.
   private RefreshTask getOrCreateRefreshTask() {
-    synchronized (refreshLock) {
-      if (refreshTask != null) {
-        // An existing refresh task is already in progress. Return a NEW RefreshTask instance with
-        // the existing task, but set isNew to false. This indicates to the caller that a new
-        // refresh task was NOT created.
-        return new RefreshTask(refreshTask.task, false);
-      }
+    if (refreshTask != null) {
+      // An existing refresh task is already in progress. Return a NEW RefreshTask instance with
+      // the existing task, but set isNew to false. This indicates to the caller that a new
+      // refresh task was NOT created.
+      return new RefreshTask(refreshTask.task, false);
+    }
 
-      final ListenableFutureTask<IntermediateCredentials> task =
-          ListenableFutureTask.create(this::fetchIntermediateCredentials);
+    final ListenableFutureTask<IntermediateCredentials> task =
+        ListenableFutureTask.create(this::fetchIntermediateCredentials);
 
-      // Store the new refresh task in the refreshTask field before returning. This ensures that
-      // subsequent calls to this method will return the existing task while it's still in progress.
-      refreshTask = new RefreshTask(task, true);
-      return refreshTask;
-    }
+    // Store the new refresh task in the refreshTask field before returning. This ensures that
+    // subsequent calls to this method will return the existing task while it's still in progress.
+    refreshTask = new RefreshTask(task, true);
+    return refreshTask;
   }
 
   /**

google-auth-library-java/cab-token-generator/javatests/com/google/auth/credentialaccessboundary/ClientSideCredentialAccessBoundaryFactoryTest.java

@@ -325,7 +325,6 @@ void refreshCredentialsIfRequired_blockingMultiThread() throws IOException, Inte
   }
 
   @Test
-  @Disabled("Flaky test: https://github.com/googleapis/google-cloud-java/issues/12871")
   void refreshCredentialsIfRequired_asyncMultiThread() throws IOException, InterruptedException {
     final ClientSideCredentialAccessBoundaryFactory factory =
         getClientSideCredentialAccessBoundaryFactory(RefreshType.ASYNC);