Skip to content

Commit 1a5753f

Browse files
committed
Refreshes that failed to schedule are not entered into cooldown.
1 parent 58525bd commit 1a5753f

3 files changed

Lines changed: 46 additions & 11 deletions

File tree

google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/RegionalAccessBoundary.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ static RegionalAccessBoundary refresh(
199199

200200
HttpRequestFactory requestFactory = transportFactory.create().createRequestFactory();
201201
HttpRequest request = requestFactory.buildGetRequest(new GenericUrl(url));
202+
// Disable automatic logging by google-http-java-client to prevent leakage of sensitive tokens.
203+
request.setLoggingEnabled(false);
202204
request.getHeaders().setAuthorization("Bearer " + accessToken.getTokenValue());
203205

204206
// Add retry logic

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ final class RegionalAccessBoundaryManager {
6464
* The default maximum elapsed time in milliseconds for retrying Regional Access Boundary lookup
6565
* requests.
6666
*/
67-
private static final int DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS = 60000;
67+
static final int DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS = 60000;
6868

6969
/**
7070
* cachedRAB uses AtomicReference to provide thread-safe, lock-free access to the cached data for
@@ -94,7 +94,7 @@ final class RegionalAccessBoundaryManager {
9494
private static final int EXECUTOR_POOL_SIZE = 5;
9595
private static final int EXECUTOR_QUEUE_CAPACITY = 100;
9696

97-
private static final ExecutorService EXECUTOR;
97+
private static final ExecutorService DEFAULT_SHARED_EXECUTOR;
9898

9999
static {
100100
ThreadPoolExecutor executor =
@@ -112,25 +112,33 @@ final class RegionalAccessBoundaryManager {
112112
// Allow core threads to time out so the executor can shrink to 0 when idle.
113113
// Ensures threads are released when idle to avoid unnecessary resource usage.
114114
executor.allowCoreThreadTimeOut(true);
115-
EXECUTOR = executor;
115+
DEFAULT_SHARED_EXECUTOR = executor;
116116
}
117117

118118
private final transient Clock clock;
119119
private final int maxRetryElapsedTimeMillis;
120+
private final ExecutorService executor;
120121

121122
/**
122123
* Creates a new RegionalAccessBoundaryManager with the default retry timeout of 60 seconds.
123124
*
124125
* @param clock The clock to use for cooldown and expiration checks.
125126
*/
126127
RegionalAccessBoundaryManager(Clock clock) {
127-
this(clock, DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS);
128+
this(clock, DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS, DEFAULT_SHARED_EXECUTOR);
128129
}
129130

130131
@VisibleForTesting
131132
RegionalAccessBoundaryManager(Clock clock, int maxRetryElapsedTimeMillis) {
133+
this(clock, maxRetryElapsedTimeMillis, DEFAULT_SHARED_EXECUTOR);
134+
}
135+
136+
@VisibleForTesting
137+
RegionalAccessBoundaryManager(
138+
Clock clock, int maxRetryElapsedTimeMillis, ExecutorService executor) {
132139
this.clock = clock != null ? clock : Clock.SYSTEM;
133140
this.maxRetryElapsedTimeMillis = maxRetryElapsedTimeMillis;
141+
this.executor = executor;
134142
}
135143

136144
/**
@@ -203,12 +211,15 @@ void triggerAsyncRefresh(
203211
};
204212

205213
try {
206-
EXECUTOR.submit(refreshTask);
214+
this.executor.submit(refreshTask);
207215
} catch (Exception | Error e) {
208216
// If scheduling fails (e.g., RejectedExecutionException, OutOfMemoryError for threads),
209217
// the task's finally block will never execute. We must release the lock here.
210-
handleRefreshFailure(
211-
new Exception("Regional Access Boundary background refresh failed to schedule", e));
218+
LoggingUtils.log(
219+
LOGGER_PROVIDER,
220+
java.util.logging.Level.WARNING,
221+
null,
222+
"Regional Access Boundary background refresh failed to schedule: " + e.getMessage());
212223
future.setException(e);
213224
refreshFuture.set(null);
214225
}

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

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,19 @@ public void testExecutorQueueCapacityLimit() throws Exception {
236236
int queueCapacity = 100;
237237
int totalCapacity = poolSize + queueCapacity;
238238

239+
java.util.concurrent.ThreadPoolExecutor testExecutor =
240+
new java.util.concurrent.ThreadPoolExecutor(
241+
poolSize,
242+
poolSize,
243+
1,
244+
java.util.concurrent.TimeUnit.HOURS,
245+
new java.util.concurrent.LinkedBlockingQueue<>(queueCapacity),
246+
r -> {
247+
Thread t = new Thread(r, "test-RAB-refresh");
248+
t.setDaemon(true);
249+
return t;
250+
});
251+
239252
CountDownLatch latch = new CountDownLatch(1);
240253

241254
java.io.InputStream blockingStream =
@@ -270,20 +283,29 @@ public int read() throws java.io.IOException {
270283

271284
RegionalAccessBoundaryManager[] managers = new RegionalAccessBoundaryManager[totalCapacity];
272285
for (int i = 0; i < totalCapacity; i++) {
273-
managers[i] = new RegionalAccessBoundaryManager(testClock);
286+
managers[i] =
287+
new RegionalAccessBoundaryManager(
288+
testClock,
289+
RegionalAccessBoundaryManager.DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS,
290+
testExecutor);
274291
managers[i].triggerAsyncRefresh(transportFactory, provider, token);
275292
}
276293

277-
RegionalAccessBoundaryManager extraManager = new RegionalAccessBoundaryManager(testClock);
294+
RegionalAccessBoundaryManager extraManager =
295+
new RegionalAccessBoundaryManager(
296+
testClock,
297+
RegionalAccessBoundaryManager.DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS,
298+
testExecutor);
278299
assertFalse(extraManager.isCooldownActive());
279300

280301
extraManager.triggerAsyncRefresh(transportFactory, provider, token);
281302

282-
assertTrue(
303+
assertFalse(
283304
extraManager.isCooldownActive(),
284-
"106th task should have been rejected and entered cooldown");
305+
"106th task should NOT have entered cooldown on scheduling failure");
285306

286307
latch.countDown();
308+
testExecutor.shutdownNow();
287309
}
288310

289311
private static class TestClock implements Clock {

0 commit comments

Comments
 (0)