Skip to content

Commit 5c41934

Browse files
committed
Executor is no longer used to execute RAB refresh.
1 parent 4f4465a commit 5c41934

5 files changed

Lines changed: 20 additions & 79 deletions

File tree

oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -353,13 +353,9 @@ final RegionalAccessBoundary getRegionalAccessBoundary() {
353353
*
354354
* @param uri The URI of the outbound request.
355355
* @param token The access token to use for the refresh.
356-
* @param executor The executor to use for the background task. If null, a new thread is created.
357356
* @throws IOException If getting the universe domain fails.
358357
*/
359-
void refreshRegionalAccessBoundaryIfExpired(
360-
@Nullable URI uri,
361-
@Nullable AccessToken token,
362-
@Nullable java.util.concurrent.Executor executor)
358+
void refreshRegionalAccessBoundaryIfExpired(@Nullable URI uri, @Nullable AccessToken token)
363359
throws IOException {
364360
if (!(this instanceof RegionalAccessBoundaryProvider)
365361
|| !RegionalAccessBoundary.isEnabled()
@@ -388,7 +384,7 @@ void refreshRegionalAccessBoundaryIfExpired(
388384
}
389385

390386
regionalAccessBoundaryManager.triggerAsyncRefresh(
391-
transportFactory, (RegionalAccessBoundaryProvider) this, token, executor);
387+
transportFactory, (RegionalAccessBoundaryProvider) this, token);
392388
}
393389

394390
/**
@@ -397,12 +393,9 @@ void refreshRegionalAccessBoundaryIfExpired(
397393
*
398394
* @param uri The URI of the outbound request.
399395
* @param requestMetadata The request metadata containing the authorization header.
400-
* @param executor The executor to use for the background task.
401396
*/
402397
void refreshRegionalAccessBoundaryWithSelfSignedJwtIfExpired(
403-
@Nullable URI uri,
404-
Map<String, List<String>> requestMetadata,
405-
@Nullable java.util.concurrent.Executor executor) {
398+
@Nullable URI uri, Map<String, List<String>> requestMetadata) {
406399
List<String> authHeaders = requestMetadata.get(AuthHttpConstants.AUTHORIZATION);
407400
if (authHeaders != null && !authHeaders.isEmpty()) {
408401
String authHeader = authHeaders.get(0);
@@ -411,7 +404,7 @@ void refreshRegionalAccessBoundaryWithSelfSignedJwtIfExpired(
411404
// Use a null expiration as JWTs are short-lived anyway.
412405
AccessToken wrappedToken = new AccessToken(tokenValue, null);
413406
try {
414-
refreshRegionalAccessBoundaryIfExpired(uri, wrappedToken, executor);
407+
refreshRegionalAccessBoundaryIfExpired(uri, wrappedToken);
415408
} catch (IOException e) {
416409
// Ignore failure in async refresh trigger.
417410
}
@@ -436,7 +429,7 @@ public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException
436429
metadata = addRegionalAccessBoundaryToRequestMetadata(uri, metadata);
437430
try {
438431
// Sets off an async refresh for request-metadata.
439-
refreshRegionalAccessBoundaryIfExpired(uri, getAccessToken(), null);
432+
refreshRegionalAccessBoundaryIfExpired(uri, getAccessToken());
440433
} catch (IOException e) {
441434
// Ignore failure in async refresh trigger.
442435
}
@@ -466,7 +459,7 @@ public void getRequestMetadata(
466459
public void onSuccess(Map<String, List<String>> metadata) {
467460
metadata = addRegionalAccessBoundaryToRequestMetadata(uri, metadata);
468461
try {
469-
refreshRegionalAccessBoundaryIfExpired(uri, getAccessToken(), executor);
462+
refreshRegionalAccessBoundaryIfExpired(uri, getAccessToken());
470463
} catch (IOException e) {
471464
// Ignore failure in async refresh trigger.
472465
}

oauth2_http/java/com/google/auth/oauth2/RegionalAccessBoundaryManager.java

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,11 @@ RegionalAccessBoundary getCachedRAB() {
121121
* @param transportFactory The HTTP transport factory to use for the lookup.
122122
* @param provider The provider used to retrieve the lookup endpoint URL.
123123
* @param accessToken The access token for authentication.
124-
* @param executor The executor to use for the background task. If null, a new thread is created.
125124
*/
126125
void triggerAsyncRefresh(
127126
final HttpTransportFactory transportFactory,
128127
final RegionalAccessBoundaryProvider provider,
129-
final AccessToken accessToken,
130-
@Nullable final java.util.concurrent.Executor executor) {
128+
final AccessToken accessToken) {
131129
if (isCooldownActive()) {
132130
return;
133131
}
@@ -163,18 +161,14 @@ void triggerAsyncRefresh(
163161
};
164162

165163
try {
166-
if (executor != null) {
167-
executor.execute(refreshTask);
168-
} else {
169-
// We use new Thread() here instead of
170-
// CompletableFuture.runAsync() (which uses ForkJoinPool.commonPool()).
171-
// This avoids consuming CPU resources since
172-
// The common pool has a small, fixed number of threads designed for
173-
// CPU-bound tasks.
174-
Thread refreshThread = new Thread(refreshTask, "RAB-refresh-thread");
175-
refreshThread.setDaemon(true);
176-
refreshThread.start();
177-
}
164+
// We use new Thread() here instead of
165+
// CompletableFuture.runAsync() (which uses ForkJoinPool.commonPool()).
166+
// This avoids consuming CPU resources since
167+
// The common pool has a small, fixed number of threads designed for
168+
// CPU-bound tasks.
169+
Thread refreshThread = new Thread(refreshTask, "RAB-refresh-thread");
170+
refreshThread.setDaemon(true);
171+
refreshThread.start();
178172
} catch (Exception | Error e) {
179173
// If scheduling fails (e.g., RejectedExecutionException, OutOfMemoryError for threads),
180174
// the task's finally block will never execute. We must release the lock here.

oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1148,7 +1148,7 @@ private Map<String, List<String>> getRequestMetadataWithSelfSignedJwt(URI uri)
11481148

11491149
Map<String, List<String>> requestMetadata = jwtCredentials.getRequestMetadata(null);
11501150
requestMetadata = addRegionalAccessBoundaryToRequestMetadata(uri, requestMetadata);
1151-
refreshRegionalAccessBoundaryWithSelfSignedJwtIfExpired(uri, requestMetadata, null);
1151+
refreshRegionalAccessBoundaryWithSelfSignedJwtIfExpired(uri, requestMetadata);
11521152
return addQuotaProjectIdToRequestMetadata(quotaProjectId, requestMetadata);
11531153
}
11541154

oauth2_http/javatests/com/google/auth/oauth2/GoogleCredentialsTest.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,10 +1247,7 @@ public AccessToken refreshAccessToken() throws IOException {
12471247

12481248
@Override
12491249
void refreshRegionalAccessBoundaryIfExpired(
1250-
@Nullable URI uri,
1251-
@Nullable AccessToken token,
1252-
@Nullable java.util.concurrent.Executor executor)
1253-
throws IOException {
1250+
@Nullable URI uri, @Nullable AccessToken token) throws IOException {
12541251
throw new IOException("Simulated RAB failure");
12551252
}
12561253
};
@@ -1272,10 +1269,7 @@ public AccessToken refreshAccessToken() throws IOException {
12721269

12731270
@Override
12741271
void refreshRegionalAccessBoundaryIfExpired(
1275-
@Nullable URI uri,
1276-
@Nullable AccessToken token,
1277-
@Nullable java.util.concurrent.Executor executor)
1278-
throws IOException {
1272+
@Nullable URI uri, @Nullable AccessToken token) throws IOException {
12791273
throw new IOException("Simulated RAB failure");
12801274
}
12811275
};

oauth2_http/javatests/com/google/auth/oauth2/RegionalAccessBoundaryTest.java

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public void testManagerTriggersRefreshInGracePeriod() throws InterruptedExceptio
153153
RegionalAccessBoundaryManager manager = new RegionalAccessBoundaryManager(testClock);
154154

155155
// 1. Let's first get a RAB into the cache
156-
manager.triggerAsyncRefresh(transportFactory, provider, token, null);
156+
manager.triggerAsyncRefresh(transportFactory, provider, token);
157157

158158
// Wait for it to be cached
159159
int retries = 0;
@@ -184,7 +184,7 @@ public void testManagerTriggersRefreshInGracePeriod() throws InterruptedExceptio
184184
HttpTransportFactory transportFactory2 = () -> transport2;
185185

186186
// 4. Trigger refresh - should start because we are in grace period
187-
manager.triggerAsyncRefresh(transportFactory2, provider, token, null);
187+
manager.triggerAsyncRefresh(transportFactory2, provider, token);
188188

189189
// 5. Wait for background refresh to complete
190190
// We expect the cached RAB to eventually change to newerEncoded
@@ -205,46 +205,6 @@ public void testManagerTriggersRefreshInGracePeriod() throws InterruptedExceptio
205205
assertEquals(newerEncoded, resultRab.getEncodedLocations());
206206
}
207207

208-
@Test
209-
public void testManagerReleasesLockOnSchedulingFailure() {
210-
RegionalAccessBoundaryManager manager = new RegionalAccessBoundaryManager(testClock);
211-
HttpTransportFactory transportFactory = () -> new MockHttpTransport();
212-
RegionalAccessBoundaryProvider provider = () -> "https://dummy";
213-
AccessToken token =
214-
new AccessToken("token", new java.util.Date(System.currentTimeMillis() + 10 * 3600000L));
215-
216-
java.util.concurrent.Executor rejectingExecutor =
217-
new java.util.concurrent.Executor() {
218-
@Override
219-
public void execute(Runnable command) {
220-
throw new java.util.concurrent.RejectedExecutionException("Simulated rejection");
221-
}
222-
};
223-
224-
manager.triggerAsyncRefresh(transportFactory, provider, token, rejectingExecutor);
225-
226-
// After rejection, the lock should be released, but it should be in cooldown.
227-
assertTrue(manager.isCooldownActive());
228-
229-
// Advance the clock to bypass cooldown
230-
testClock.set(
231-
testClock.currentTimeMillis() + RegionalAccessBoundaryManager.MAX_COOLDOWN_MILLIS + 1000);
232-
assertFalse(manager.isCooldownActive());
233-
234-
// Schedule again with a valid executor to prove the lock was released.
235-
java.util.concurrent.atomic.AtomicBoolean taskRan =
236-
new java.util.concurrent.atomic.AtomicBoolean(false);
237-
java.util.concurrent.Executor workingExecutor =
238-
new java.util.concurrent.Executor() {
239-
@Override
240-
public void execute(Runnable command) {
241-
taskRan.set(true);
242-
}
243-
};
244-
manager.triggerAsyncRefresh(transportFactory, provider, token, workingExecutor);
245-
assertTrue(taskRan.get());
246-
}
247-
248208
private static class TestClock implements Clock {
249209
private final AtomicLong currentTime = new AtomicLong(System.currentTimeMillis());
250210

0 commit comments

Comments
 (0)