Shutdown newFixedThreadPool and newSingleThreadExecutor.
The javadoc of newFixedThreadPool says:
> The threads in the pool will exist until it is explicitly shutdown.
The javadoc of newSingleThreadExecutor says:
> Unlike the otherwise equivalent newFixedThreadPool(1) the returned
> executor is guaranteed not to be reconfigurable to use additional
> threads.
So we have to manually shut them down, or we will leak them.
Bug: 259472777
Test: atest ArtServiceTests
Ignore-AOSP-First: ART Services.
Change-Id: I7642ea50180f95c61e874d069cecab2a63a2dbbe
diff --git a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
index b0d5fb3..787c3a2 100644
--- a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
+++ b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
@@ -68,6 +68,7 @@
import java.util.List;
import java.util.Objects;
import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.function.Consumer;
import java.util.stream.Collectors;
@@ -346,10 +347,15 @@
BatchOptimizeParams params = builder.build();
Utils.check(params.getOptimizeParams().getReason().equals(reason));
- return mInjector.getDexOptHelper().dexopt(snapshot, params.getPackages(),
- params.getOptimizeParams(), cancellationSignal,
- Executors.newFixedThreadPool(ReasonMapping.getConcurrencyForReason(reason)),
- processCallbackExecutor, progressCallback);
+ ExecutorService dexoptExecutor =
+ Executors.newFixedThreadPool(ReasonMapping.getConcurrencyForReason(reason));
+ try {
+ return mInjector.getDexOptHelper().dexopt(snapshot, params.getPackages(),
+ params.getOptimizeParams(), cancellationSignal, dexoptExecutor,
+ processCallbackExecutor, progressCallback);
+ } finally {
+ dexoptExecutor.shutdown();
+ }
}
/**
diff --git a/libartservice/service/java/com/android/server/art/ArtShellCommand.java b/libartservice/service/java/com/android/server/art/ArtShellCommand.java
index 95996b3..c282e8c 100644
--- a/libartservice/service/java/com/android/server/art/ArtShellCommand.java
+++ b/libartservice/service/java/com/android/server/art/ArtShellCommand.java
@@ -54,7 +54,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
-import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.stream.Collectors;
@@ -161,7 +161,7 @@
}
case "optimize-packages": {
OptimizeResult result;
- Executor executor = Executors.newSingleThreadExecutor();
+ ExecutorService executor = Executors.newSingleThreadExecutor();
try (var signal = new WithCancellationSignal(pw)) {
result = mArtManagerLocal.optimizePackages(snapshot, getNextArgRequired(),
signal.get(), executor, progress -> {
@@ -169,8 +169,10 @@
"Optimizing apps: %d%%", progress.getPercentage()));
pw.flush();
});
+ Utils.executeAndWait(executor, () -> printOptimizeResult(pw, result));
+ } finally {
+ executor.shutdown();
}
- Utils.executeAndWait(executor, () -> printOptimizeResult(pw, result));
return 0;
}
case "cancel": {
diff --git a/libartservice/service/java/com/android/server/art/BackgroundDexOptJob.java b/libartservice/service/java/com/android/server/art/BackgroundDexOptJob.java
index 03f8610..e1f0619 100644
--- a/libartservice/service/java/com/android/server/art/BackgroundDexOptJob.java
+++ b/libartservice/service/java/com/android/server/art/BackgroundDexOptJob.java
@@ -44,7 +44,6 @@
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
/** @hide */
diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
index 1c56b71..dea9fda 100644
--- a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
+++ b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
@@ -79,7 +79,8 @@
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.List;
-import java.util.concurrent.Executors;
+import java.util.concurrent.ForkJoinPool;
+import java.util.function.Consumer;
import java.util.stream.Collectors;
@SmallTest
@@ -379,8 +380,8 @@
var result = mock(OptimizeResult.class);
var cancellationSignal = new CancellationSignal();
- mArtManagerLocal.setOptimizePackagesCallback(Executors.newSingleThreadExecutor(),
- (snapshot, reason, defaultPackages, builder) -> {
+ mArtManagerLocal.setOptimizePackagesCallback(
+ ForkJoinPool.commonPool(), (snapshot, reason, defaultPackages, builder) -> {
assertThat(reason).isEqualTo("bg-dexopt");
assertThat(defaultPackages).containsExactly(PKG_NAME, PKG_NAME_SYS_UI);
builder.setPackages(List.of(PKG_NAME)).setOptimizeParams(params);
@@ -402,8 +403,8 @@
var result = mock(OptimizeResult.class);
var cancellationSignal = new CancellationSignal();
- mArtManagerLocal.setOptimizePackagesCallback(Executors.newSingleThreadExecutor(),
- (snapshot, reason, defaultPackages, builder) -> {
+ mArtManagerLocal.setOptimizePackagesCallback(
+ ForkJoinPool.commonPool(), (snapshot, reason, defaultPackages, builder) -> {
builder.setPackages(List.of(PKG_NAME)).setOptimizeParams(params);
});
mArtManagerLocal.clearOptimizePackagesCallback();
@@ -423,8 +424,8 @@
var params = new OptimizeParams.Builder("first-boot").build();
var cancellationSignal = new CancellationSignal();
- mArtManagerLocal.setOptimizePackagesCallback(Executors.newSingleThreadExecutor(),
- (snapshot, reason, defaultPackages, builder) -> {
+ mArtManagerLocal.setOptimizePackagesCallback(
+ ForkJoinPool.commonPool(), (snapshot, reason, defaultPackages, builder) -> {
builder.setOptimizeParams(params);
});
diff --git a/libartservice/service/javatests/com/android/server/art/DexOptHelperTest.java b/libartservice/service/javatests/com/android/server/art/DexOptHelperTest.java
index 59af8be..f0a0cc7 100644
--- a/libartservice/service/javatests/com/android/server/art/DexOptHelperTest.java
+++ b/libartservice/service/javatests/com/android/server/art/DexOptHelperTest.java
@@ -54,6 +54,7 @@
import com.android.server.pm.pkg.PackageState;
import com.android.server.pm.pkg.SharedLibrary;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -100,7 +101,7 @@
private AndroidPackage mPkgLib4;
private AndroidPackage mPkgLibbaz;
private CancellationSignal mCancellationSignal;
- private ExecutorService mExecutor = Executors.newSingleThreadExecutor();
+ private ExecutorService mExecutor;
private List<DexContainerFileOptimizeResult> mPrimaryResults;
private List<DexContainerFileOptimizeResult> mSecondaryResults;
private Config mConfig;
@@ -118,6 +119,7 @@
lenient().when(mAhm.isOatArtifactDeletionEnabled()).thenReturn(true);
mCancellationSignal = new CancellationSignal();
+ mExecutor = Executors.newSingleThreadExecutor();
mConfig = new Config();
preparePackagesAndLibraries();
@@ -151,6 +153,11 @@
mDexOptHelper = new DexOptHelper(mInjector);
}
+ @After
+ public void tearDown() {
+ mExecutor.shutdown();
+ }
+
@Test
public void testDexopt() throws Exception {
// Only package libbaz fails.
diff --git a/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java b/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java
index 1c879d7..1f7398b3 100644
--- a/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java
+++ b/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java
@@ -57,7 +57,7 @@
import java.util.ArrayList;
import java.util.List;
-import java.util.concurrent.Executors;
+import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.Future;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
@@ -532,8 +532,7 @@
.cancel();
Future<List<DexContainerFileOptimizeResult>> results =
- Executors.newSingleThreadExecutor().submit(
- () -> { return mPrimaryDexOptimizer.dexopt(); });
+ ForkJoinPool.commonPool().submit(() -> { return mPrimaryDexOptimizer.dexopt(); });
assertThat(dexoptStarted.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue();
diff --git a/libartservice/service/javatests/com/android/server/art/UtilsTest.java b/libartservice/service/javatests/com/android/server/art/UtilsTest.java
index 93e927b..ff09dc4 100644
--- a/libartservice/service/javatests/com/android/server/art/UtilsTest.java
+++ b/libartservice/service/javatests/com/android/server/art/UtilsTest.java
@@ -41,7 +41,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
+import java.util.concurrent.ForkJoinPool;
@SmallTest
@RunWith(AndroidJUnit4.class)
@@ -178,7 +178,7 @@
@Test
public void testExecuteAndWait() {
- Executor executor = Executors.newSingleThreadExecutor();
+ Executor executor = ForkJoinPool.commonPool();
List<String> results = new ArrayList<>();
Utils.executeAndWait(executor, () -> {
try {
@@ -193,7 +193,7 @@
@Test(expected = IllegalArgumentException.class)
public void testExecuteAndWaitPropagatesException() {
- Executor executor = Executors.newSingleThreadExecutor();
+ Executor executor = ForkJoinPool.commonPool();
Utils.executeAndWait(executor, () -> { throw new IllegalArgumentException(); });
}
}