Skip to content

s2a: use AdvancedTls. #4004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rmehta19 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

s2a: use AdvancedTls. #4004

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions ...java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
import io.grpc.TlsChannelCredentials;
import io.grpc.alts.GoogleDefaultChannelCredentials;
import io.grpc.auth.MoreCallCredentials;
import io.grpc.util.AdvancedTlsX509KeyManager;
import io.grpc.util.AdvancedTlsX509TrustManager;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Method;
Expand All @@ -78,6 +80,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
Expand Down Expand Up @@ -556,13 +559,32 @@ ChannelCredentials buildS2AChannelCredentials(
*/
@VisibleForTesting
ChannelCredentials createMtlsToS2AChannelCredentials(
File trustBundle, File privateKey, File certChain) throws IOException {
File trustBundle, File privateKey, File certChain)
throws IOException, GeneralSecurityException {
if (trustBundle == null || privateKey == null || certChain == null) {
return null;
}
AdvancedTlsX509KeyManager keyManager = new AdvancedTlsX509KeyManager();
ScheduledExecutorService keyManagerExecutor = Executors.newSingleThreadScheduledExecutor(
r -> {
Thread t = new Thread(r, "s2a-key-manager-updater");
t.setDaemon(true);
return t;
});

keyManager.updateIdentityCredentials(certChain, privateKey, 1, TimeUnit.HOURS, keyManagerExecutor);
AdvancedTlsX509TrustManager trustManager = AdvancedTlsX509TrustManager.newBuilder().build();
ScheduledExecutorService trustManagerExecutor = Executors.newSingleThreadScheduledExecutor(
r -> {
Thread t = new Thread(r, "s2a-trust-manager-updater");
t.setDaemon(true);
return t;
});
Comment on lines +568 to +582
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Creating new ScheduledExecutorService instances within createMtlsToS2AChannelCredentials on each call will lead to a resource leak. This method may be called multiple times during the lifetime of a client (e.g., for each channel in a pool, and when channels are refreshed), but the created executors are never shut down. This will cause threads to accumulate over time.

To fix this, you should use shared ScheduledExecutorService instances. A recommended approach is to define them as static final fields in the InstantiatingGrpcChannelProvider class and reuse them here. Since the threads are already configured as daemons, they won't block JVM exit, but sharing them is crucial to prevent resource leaks within a long-running application.

Additionally, the logic for creating the executors is duplicated. You could extract it into a helper method. You might also consider if a single shared executor is sufficient for both the key manager and the trust manager, which would further reduce resource usage.


trustManager.updateTrustCredentials(trustBundle, 1, TimeUnit.HOURS, trustManagerExecutor);
return TlsChannelCredentials.newBuilder()
.keyManager(privateKey, certChain)
.trustManager(trustBundle)
.keyManager(keyManager)
.trustManager(trustManager)
.build();
}

Expand Down
Loading