From b1f8d47d8c9b3d477971cfa581747558c5805f4a Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 4 Mar 2024 05:46:01 -0800 Subject: [PATCH] [7.1.0] Fix a flaky test by avoiding leaking the eager capability RPC thread. Arguably a better fix would be for every test case to properly dispose of the RemoteModule via afterCommand() and making sure that the leak is stopped there. Leave a TODO accordingly. PiperOrigin-RevId: 612417631 Change-Id: I2d6e6e94b405eb777a884c6e10f9d3f9a086fffc --- .../com/google/devtools/build/lib/remote/RemoteModule.java | 1 + .../google/devtools/build/lib/remote/RemoteModuleTest.java | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index 4cac1fdc44bacf..cce90c4956e436 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -695,6 +695,7 @@ private static ReferenceCountedChannel createChannel( requirement), maxConnections); // Eagerly start creating the channel and verifying the capabilities in the background. + // TODO(tjgq): Make sure this task doesn't linger beyond afterCommand(). var unused = executorService.submit( () -> { diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java index 994f23fcae5df4..ccc39fa1f1c0a4 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java @@ -241,6 +241,12 @@ public void testVerifyCapabilities_none() throws Exception { // Remote downloader uses Remote Asset API, and Bazel doesn't have any capability requirement // on the endpoint. Expecting the request count is 0. assertThat(cacheCapabilitiesImpl.getRequestCount()).isEqualTo(0); + + // Retrieve the execution capabilities so that the asynchronous task that eagerly requests + // them doesn't leak and accidentally interfere with other test cases. + assertThat(remoteModule.getActionContextProvider().getRemoteCache().getCacheCapabilities()) + .isEqualTo(EXEC_AND_CACHE_CAPS.getCacheCapabilities()); + assertCircuitBreakerInstance(); } finally { executionServer.shutdownNow();