[BUG] `ClientSecretCredential.getToken().block()` will hang when parallelism is high

[XiaofeiCao]

Describe the bug
When ClientSecretCredential.getToken().block() gets called on ForkJoinPool.commonPool with parallelism greater than the pool's configured parallelism(by default its the cpuCoreCount(-1)), it'll get stuck forever.

See IcM: https://portal.microsofticm.com/imp/v3/incidents/incident/492551519/summary
Reason they need to do auth in parallel is because they have 600+ subs, and they need one TokenCredential for each of the subs.

Exception or Stack Trace

Thread dump

Details

"ForkJoinPool.commonPool-worker-1" #63 daemon prio=5 os_prio=0 cpu=400.78ms elapsed=3042.78s allocated=33882K defined_classes=188 tid=0x00007f158c2c4790 nid=0x73 waiting on condition [0x00007f157a5f6000] java.lang.Thread.State: WAITING (parking) at jdk.internal.misc.Unsafe.park(java.base@17.0.10/Native Method) - parking to wait for <0x000000067aa2f2d8> (a java.util.concurrent.CountDownLatch$Sync) at java.util.concurrent.locks.LockSupport.park(java.base@17.0.10/LockSupport.java:211) at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(java.base@17.0.10/AbstractQueuedSynchronizer.java:715) at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(java.base@17.0.10/AbstractQueuedSynchronizer.java:1047) at java.util.concurrent.CountDownLatch.await(java.base@17.0.10/CountDownLatch.java:230) at reactor.core.publisher.BlockingSingleSubscriber.blockingGet(BlockingSingleSubscriber.java:87) at reactor.core.publisher.Flux.blockLast(Flux.java:2761) at com.azure.core.util.paging.ContinuablePagedByIteratorBase.requestPage(ContinuablePagedByIteratorBase.java:102) - locked <0x000000067aa2f370> (a com.azure.core.util.paging.ContinuablePagedByItemIterable$ContinuablePagedByItemIterator) at com.azure.core.util.paging.ContinuablePagedByItemIterable$ContinuablePagedByItemIterator.(ContinuablePagedByItemIterable.java:75) at com.azure.core.util.paging.ContinuablePagedByItemIterable.iterator(ContinuablePagedByItemIterable.java:55) at com.azure.core.util.paging.ContinuablePagedIterable.iterator(ContinuablePagedIterable.java:141) at com.azure.resourcemanager.resources.fluentcore.utils.PagedConverter$PagedIterableImpl.iterator(PagedConverter.java:225) at com.hybris.cloud.automation.azureintegration.AzureSDKService.getAzureSubscriptions(AzureSDKService.java:547) at com.hybris.cloud.automation.azureintegration.AzureSDKService.check(AzureSDKService.java:110) at com.hybris.cloud.automation.azureintegration.spn.AzureSubscriptionAndSpnsDefaultSpnConnectionCheck.status(AzureSubscriptionAndSpnsDefaultSpnConnectionCheck.java:71) at com.hybris.cloud.automation.azureintegration.spn.AzureSubscriptionAndSpnsDefaultSpnConnectionCheck.status(AzureSubscriptionAndSpnsDefaultSpnConnectionCheck.java:30) at com.hybris.cloud.automation.cmdbintegration.connectioncheck.ConnectionCheckService.check(ConnectionCheckService.java:127) at com.hybris.cloud.automation.cmdbintegration.connectioncheck.ConnectionCheckService.lambda$connectionCheck$4(ConnectionCheckService.java:104) at com.hybris.cloud.automation.cmdbintegration.connectioncheck.ConnectionCheckService$$Lambda$2902/0x0000000801d60518.apply(Unknown Source) at java.util.stream.Collectors.lambda$toMap$68(java.base@17.0.10/Collectors.java:1674) at java.util.stream.Collectors$$Lambda$859/0x0000000800b71668.accept(java.base@17.0.10/Unknown Source) at java.util.stream.ReduceOps$3ReducingSink.accept(java.base@17.0.10/ReduceOps.java:169) at java.util.stream.ReferencePipeline$2$1.accept(java.base@17.0.10/ReferencePipeline.java:179) at java.util.Spliterators$ArraySpliterator.forEachRemaining(java.base@17.0.10/Spliterators.java:992) at java.util.stream.AbstractPipeline.copyInto(java.base@17.0.10/AbstractPipeline.java:509) at java.util.stream.AbstractPipeline.wrapAndCopyInto(java.base@17.0.10/AbstractPipeline.java:499) at java.util.stream.ReduceOps$ReduceTask.doLeaf(java.base@17.0.10/ReduceOps.java:960) at java.util.stream.ReduceOps$ReduceTask.doLeaf(java.base@17.0.10/ReduceOps.java:934) at java.util.stream.AbstractTask.compute(java.base@17.0.10/AbstractTask.java:327) at java.util.concurrent.CountedCompleter.exec(java.base@17.0.10/CountedCompleter.java:754) at java.util.concurrent.ForkJoinTask.doExec(java.base@17.0.10/ForkJoinTask.java:373) at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(java.base@17.0.10/ForkJoinPool.java:1182) at java.util.concurrent.ForkJoinPool.scan(java.base@17.0.10/ForkJoinPool.java:1655) at java.util.concurrent.ForkJoinPool.runWorker(java.base@17.0.10/ForkJoinPool.java:1622) at java.util.concurrent.ForkJoinWorkerThread.run(java.base@17.0.10/ForkJoinWorkerThread.java:165)

To Reproduce
See this test: https://github.com/XiaofeiCao/azure-identity-test/blob/main/src/test/java/com/xiaofei/AzureIdentityBlockTest.java
One can clone and repo and run the test.

Code Snippet

IntStream.range(1, parallelism).parallel().forEach(iii -> {
  credential.getToken(new TokenRequestContext().addScopes("https://management.core.windows.net//.default")).block();
  // use getTokenSync will also resolve the hang
  //            credential.getTokenSync(new TokenRequestContext().addScopes("https://management.core.windows.net//.default"));
  
  System.out.println("fetched for " + iii);
});

Expected behavior
We suspect that it's due to caller threads and ClientSecretCredential's execution threads are shared from the same ForkJoinPool.commonPool, as making the ClientSecretCredential run on custom thread pool will resolve the hang. Though currently we are not 100% sure it's the exact case that customer experienced.
May need your guidance.

Screenshots
If applicable, add screenshots to help explain your problem.

Setup (please complete the following information):

• OS: win or mac
• IDE: N/A
• Library/Libraries: com.azure:azure-identity:1.12.0
• Java version: 11 or 17
• App Server/Environment: Tomcat or local JVM
• Frameworks: Spring or none

If you suspect a dependency version mismatch (e.g. you see NoClassDefFoundError, NoSuchMethodError or similar), please check out Troubleshoot dependency version conflict article first. If it doesn't provide solution for the problem, please provide:

• verbose dependency tree (mvn dependency:tree -Dverbose)
• exception message, full stack trace, and any available logs

Additional context
Add any other context about the problem here.

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

• Bug Description Added
• Repro Steps Added
• Setup information Added

/cc @weidongxu-microsoft @dennis-menge

[weidongxu-microsoft]

From the thread dump, there is only 3 ForkJoinPool.commonPool thread in the system.

All of them is under blocking call of AzureSDKService.getAzureSubscriptions (which would call to management.azure.com, that could take around 100ms to 1 second as it send HTTP request)

Still checking whether customer put the HTTP call to ForkJoinPool.commonPool thread and causes the bottleneck there.

But the azure-identity default using ForkJoinPool.commonPool could be a problem as well. As the HTTP call (to management.azure.com) already on ForkJoinPool.commonPool now need another thread from ForkJoinPool.commonPool to retrieve auth token (another HTTP call to login.microsoftonline.com).

Customer has lots of tenants, so many of those requests do indeed need to retrieve a new auth token (instead of use the cache).

We'd like advice from azure-identity, whether customer should use a different ExecutorService, if there is already competition on ForkJoinPool.commonPool

[dennis-menge]

Repro the behavior without any SDK code on machines with less than 15 cores

    private void repro() {
        IntStream.range(1, 15).parallel().forEach(iii -> {
            Mono.empty()
                    .switchIfEmpty(Mono.defer(() -> Mono.fromFuture(() -> CompletableFuture.supplyAsync(() -> {
                        try {
                            Thread.sleep(1000);
                        } catch (InterruptedException e) {
                            throw new RuntimeException(e);
                        }
                        System.out.println("I am alive " + iii + " " + Thread.currentThread().getName());
                        return null;
                    })))
            ).block();
        });
    }

[g2vinay]

While we work with Msal to add a more resilient threading model.

Alternatively user can specify the executor service directly on the credential.

ExecutorService executorService = Executors.newCachedThreadPool(); // Get a non-default thread pool.  

try {  
    final ClientSecretCredential clientSecretCredential = new ClientSecretCredentialBuilder()  
        .clientId("<client-ID>")  
        .clientSecret("<client-secret>")  
        .tenantId("<tenant-ID>")  
        .executorService(executorService) // ExecutorService explicitly given to CredentialBuilder.  
        .build();  

    // Build a client(s) that uses the Credential object created above.  
    final SecretClient secretClient = new SecretClientBuilder()  
        .vaultUrl("<your-kay-vault-URI>")  
        .credential(clientSecretCredential)  
        .buildClient();  

    // Execute all desired operations.  
    final KeyVaultSecret secret = secretClient.getSecret("<secret-name>");  

    System.out.printf("Retrieved secret with name \"%s\" and value \"%s\"\n", secret.getName(),  
        secret.getValue());  
} finally {  
    // This block will be reached after the above code is executed or if an uncaught exception is raised.  

    // It is important to shut down the ExecutorService after it is no longer required (i.e. once the client(s)  
    // using the Credential object with the ExecutorService will not be used any longer).  

    // Use the following if it is possible there are still operations that require authentication running  
    // asynchronously. Otherwise, it is safe to use the alternative "executorService.shutdownNow();"  
    executorService.shutdown();  
}

[github-actions]

Hi @XiaofeiCao. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.