Skip to content

Commit a3886d7

Browse files
committed
Noting down the intention of the fix
1 parent 7ad23db commit a3886d7

File tree

1 file changed

+28
-2
lines changed

1 file changed

+28
-2
lines changed

cloudfoundry-client-reactor/src/main/java/org/cloudfoundry/reactor/tokenprovider/AbstractUaaTokenProvider.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,21 +306,47 @@ private void setAuthorization(HttpHeaders headers) {
306306
}
307307

308308
private Mono<String> token(final ConnectionContext connectionContext) {
309+
/*
310+
* Beware of issue #1146!
311+
* There can be both multiple concurrent callers coming here during creation of the Mono
312+
* and there can be concurrent callers during the subscription/execution of the Mono:
313+
* - The latter is very harmful: This may lead to concurrent execution of the logic
314+
* written in requestToken and hence to multiple requests to the UAA server using
315+
* the same *value for the refresh token*! The UAA server will sequentialize them,
316+
* one will go through just as normal and a new refresh token gets issued.
317+
* The UAA server invalidates the old refresh token. The second request then arrives
318+
* with the old refresh token and gets rejected. In an earlier version this led
319+
* to caching the second request, hence to cache an error. This caused deadlocks.
320+
* - The first is only "not nice", if the second issue is resolved: It causes that
321+
* we will request two access tokens from the UAA server shortly after the other,
322+
* but having use appropriate refresh tokens sequentially. The second request
323+
* simply is to be considered waste.
324+
*
325+
* The coding below fixes both issues: It ensures that the execution of the Mono
326+
* is synchronized and it ensures that two threads arriving to fetch the JWT in a
327+
* non-caching situation does not trigger "wasteful" requests to the UAA server.
328+
*/
329+
309330
// Get or create a single-threaded scheduler for this connection context
310331
final Scheduler tokenScheduler = this.tokenSchedulers.computeIfAbsent(
311332
connectionContext,
312333
ctx -> Schedulers.newSingle("token-" + ctx.hashCode())
313334
);
314335

336+
/*
337+
* We use Mono.defer to ensure that the execution of the locking not happens
338+
* during creation of the Mono (where it is of little relevance), but
339+
* during the execution/subscription.
340+
*/
315341
return Mono.defer(() -> {
316342
// Check if there's already an active token request
317343
final Mono<String> existingRequest = this.activeTokenRequests.get(connectionContext);
318344
if (existingRequest != null) {
319-
LOGGER.debug("Reusing existing token request for connection context");
345+
LOGGER.debug("Reusing existing UAA JWT token request for connection context");
320346
return existingRequest;
321347
}
322348

323-
// Create new token request with proper synchronization and cache duration
349+
// Create new token request with proper synchronization
324350
final Mono<String> baseTokenRequest = createTokenRequest(connectionContext)
325351
.publishOn(tokenScheduler) // Ensure execution on single thread
326352
.doOnSubscribe(s -> LOGGER.debug("Starting new token request"))

0 commit comments

Comments
 (0)