xds: Fix XdsSecurityClientServerTest TrustManagerStore race

When spiffe support was added it caused tlsClientServer_useSystemRootCerts_validationContext to become flaky. This is because test execution order was important for whether the race would occur. Fixes grpc#11678
kannanjgithub · Nov 15, 2024 · 1f159d7 · 1f159d7
1 parent 4e8f7df
commit 1f159d7
Showing 1 changed file with 19 additions and 2 deletions.
diff --git a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java
@@ -97,6 +97,7 @@
 import java.util.concurrent.TimeUnit;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.TrustManagerFactory;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -687,16 +688,32 @@ public void run() {
     return settableFuture;
   }
 
-  private void setTrustStoreSystemProperties(String trustStoreFilePath) {
+  private void setTrustStoreSystemProperties(String trustStoreFilePath) throws Exception {
     System.setProperty("javax.net.ssl.trustStore", trustStoreFilePath);
     System.setProperty("javax.net.ssl.trustStorePassword", "changeit");
     System.setProperty("javax.net.ssl.trustStoreType", "JKS");
+    createDefaultTrustManager();
   }
 
-  private void clearTrustStoreSystemProperties() {
+  private void clearTrustStoreSystemProperties() throws Exception {
     System.clearProperty("javax.net.ssl.trustStore");
     System.clearProperty("javax.net.ssl.trustStorePassword");
     System.clearProperty("javax.net.ssl.trustStoreType");
+    createDefaultTrustManager();
+  }
+
+  /**
+   * Workaround the JDK's TrustManagerStore race. TrustManagerStore has a cache for the default
+   * certs based on the system properties. But updating the cache is not thread-safe and can cause a
+   * half-updated cache to appear fully-updated. When both the client and server initialize their
+   * trust store simultaneously, one can see a half-updated value. Creating the trust manager here
+   * fixes the cache while no other threads are running and thus the client and server threads won't
+   * race to update it. See https://github.com/grpc/grpc-java/issues/11678.
+   */
+  private void createDefaultTrustManager() throws Exception {
+    TrustManagerFactory factory =
+        TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+    factory.init((KeyStore) null);
   }
 
   private static class SimpleServiceImpl extends SimpleServiceGrpc.SimpleServiceImplBase {