From 7c26f98bf3d95616d516dd8cdb0ff4ef64e0b219 Mon Sep 17 00:00:00 2001 From: Jiakai Zhang Date: Fri, 3 May 2024 12:19:43 +0100 Subject: Update file GC for Pre-reboot Dexopt. - Keep ".staged" files if Pre-reboot Dexopt has run. - Remove ".staged" files if Pre-reboot Dexopt has not run. Bug: 311377497 Test: atest ArtServiceTests Test: m test-art-host-gtest-art_artd_tests Change-Id: If57e5b6af036ed02ba6f5671dee985dc6d554023 --- artd/artd.cc | 4 +- artd/artd.h | 1 + artd/artd_test.cc | 271 +++++++++++---------- artd/binder/com/android/server/art/IArtd.aidl | 3 +- artd/path_utils.cc | 16 +- artd/path_utils.h | 2 + .../com/android/server/art/ArtManagerLocal.java | 5 +- .../com/android/server/art/PreRebootDexoptJob.java | 15 ++ .../android/server/art/ArtManagerLocalTest.java | 18 +- .../android/server/art/PreRebootDexoptJobTest.java | 6 +- 10 files changed, 207 insertions(+), 134 deletions(-) diff --git a/artd/artd.cc b/artd/artd.cc index 095e78e495..f3853365fc 100644 --- a/artd/artd.cc +++ b/artd/artd.cc @@ -1250,6 +1250,7 @@ ScopedAStatus Artd::cleanup(const std::vector& in_profilesToKeep, const std::vector& in_artifactsToKeep, const std::vector& in_vdexFilesToKeep, const std::vector& in_runtimeArtifactsToKeep, + bool in_keepPreRebootStagedFiles, int64_t* _aidl_return) { RETURN_FATAL_IF_PRE_REBOOT(options_); std::unordered_set files_to_keep; @@ -1278,7 +1279,8 @@ ScopedAStatus Artd::cleanup(const std::vector& in_profilesToKeep, } *_aidl_return = 0; for (const std::string& file : ListManagedFiles(android_data, android_expand)) { - if (files_to_keep.find(file) == files_to_keep.end()) { + if (files_to_keep.find(file) == files_to_keep.end() && + (!in_keepPreRebootStagedFiles || !IsPreRebootStagedFile(file))) { LOG(INFO) << ART_FORMAT("Cleaning up obsolete file '{}'", file); *_aidl_return += GetSizeAndDeleteFile(file); } diff --git a/artd/artd.h b/artd/artd.h index 37b6d0c42f..07927104df 100644 --- a/artd/artd.h +++ b/artd/artd.h @@ -199,6 +199,7 @@ class Artd : public aidl::com::android::server::art::BnArtd { const std::vector& in_vdexFilesToKeep, const std::vector& in_runtimeArtifactsToKeep, + bool in_keepPreRebootStagedFiles, int64_t* _aidl_return) override; ndk::ScopedAStatus isInDalvikCache(const std::string& in_dexFile, bool* _aidl_return) override; diff --git a/artd/artd_test.cc b/artd/artd_test.cc index 858fd9909b..1c71ab2fbd 100644 --- a/artd/artd_test.cc +++ b/artd/artd_test.cc @@ -2094,138 +2094,165 @@ TEST_F(ArtdTest, mergeProfilesWithOptionsDumpClassesAndMethods) { CheckContent(output_profile.profilePath.tmpPath, "dump"); } -TEST_F(ArtdTest, cleanup) { - std::vector gc_removed_files; - std::vector gc_kept_files; +class ArtdCleanupTest : public ArtdTest { + protected: + void SetUp() override { + ArtdTest::SetUp(); - auto CreateGcRemovedFile = [&](const std::string& path) { + // Unmanaged files. + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/1.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.txt"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.txt"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.tmp"); + + // Files to keep. + CreateGcKeptFile(android_data_ + "/misc/profiles/cur/1/com.android.foo/primary.prof"); + CreateGcKeptFile(android_data_ + "/misc/profiles/cur/3/com.android.foo/primary.prof"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.vdex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.vdex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_data_ + "/user/0/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_data_ + "/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_expand_ + + "/123456-7890/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_data_ + + "/user/0/com.android.foo/cache/not_oat_dir/oat_primary/arm64/base.art"); + + // Files to remove. + CreateGcRemovedFile(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof"); + CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/2/com.android.foo/primary.prof"); + CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/3/com.android.bar/primary.prof"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/extra.odex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.dex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.vdex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.art"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.odex"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.vdex"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.art"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/2.odex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.bar/aaa/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + + "/user/0/com.android.different_package/cache/oat_primary/arm64/base.art"); + CreateGcRemovedFile(android_data_ + + "/user/0/com.android.foo/cache/oat_primary/arm64/different_dex.art"); + CreateGcRemovedFile(android_data_ + + "/user/0/com.android.foo/cache/oat_primary/different_isa/base.art"); + } + + void CreateGcRemovedFile(const std::string& path) { CreateFile(path); - gc_removed_files.push_back(path); - }; + gc_removed_files_.push_back(path); + } - auto CreateGcKeptFile = [&](const std::string& path) { + void CreateGcKeptFile(const std::string& path) { CreateFile(path); - gc_kept_files.push_back(path); - }; + gc_kept_files_.push_back(path); + } - // Unmanaged files. - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/1.odex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.odex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.txt"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.txt"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.tmp"); - - // Files to keep. - CreateGcKeptFile(android_data_ + "/misc/profiles/cur/1/com.android.foo/primary.prof"); - CreateGcKeptFile(android_data_ + "/misc/profiles/cur/3/com.android.foo/primary.prof"); - CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex"); - CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex"); - CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex"); - CreateGcKeptFile( - android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex"); - CreateGcKeptFile( - android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.vdex"); + void RunCleanup(bool keepPreRebootStagedFiles) { + int64_t aidl_return; + ASSERT_STATUS_OK(artd_->cleanup( + { + PrimaryCurProfilePath{ + .userId = 1, .packageName = "com.android.foo", .profileName = "primary"}, + PrimaryCurProfilePath{ + .userId = 3, .packageName = "com.android.foo", .profileName = "primary"}, + }, + { + ArtifactsPath{ + .dexPath = "/system/app/Foo/Foo.apk", .isa = "arm64", .isInDalvikCache = true}, + ArtifactsPath{ + .dexPath = android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/base.apk", + .isa = "arm64", + .isInDalvikCache = false}, + ArtifactsPath{.dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/2.apk", + .isa = "arm64", + .isInDalvikCache = false}, + }, + { + VdexPath{ + ArtifactsPath{.dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/1.apk", + .isa = "arm64", + .isInDalvikCache = false}}, + }, + { + RuntimeArtifactsPath{ + .packageName = "com.android.foo", .dexPath = "/a/b/base.apk", .isa = "arm64"}, + }, + keepPreRebootStagedFiles, + &aidl_return)); + } + + void Verify() { + for (const std::string& path : gc_removed_files_) { + EXPECT_FALSE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be removed", path); + } + + for (const std::string& path : gc_kept_files_) { + EXPECT_TRUE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be kept", path); + } + } + + private: + std::vector gc_removed_files_; + std::vector gc_kept_files_; +}; + +TEST_F(ArtdCleanupTest, cleanupKeepingPreRebootStagedFiles) { CreateGcKeptFile( android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.art"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.vdex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.art"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_data_ + "/user/0/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_data_ + "/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_expand_ + - "/123456-7890/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_data_ + - "/user/0/com.android.foo/cache/not_oat_dir/oat_primary/arm64/base.art"); - - // Files to remove. - CreateGcRemovedFile(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof"); - CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/2/com.android.foo/primary.prof"); - CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/3/com.android.bar/primary.prof"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/extra.odex"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.dex"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.vdex"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.art"); - CreateGcRemovedFile( - android_expand_ + - "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.odex"); - CreateGcRemovedFile( - android_expand_ + - "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.vdex"); - CreateGcRemovedFile( - android_expand_ + - "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.art"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.vdex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.art"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/2.odex.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.odex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.art"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.odex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.vdex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art"); - CreateGcRemovedFile(android_data_ + - "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.bar/aaa/oat/arm64/1.vdex"); - CreateGcRemovedFile(android_data_ + - "/user/0/com.android.different_package/cache/oat_primary/arm64/base.art"); - CreateGcRemovedFile(android_data_ + - "/user/0/com.android.foo/cache/oat_primary/arm64/different_dex.art"); - CreateGcRemovedFile(android_data_ + - "/user/0/com.android.foo/cache/oat_primary/different_isa/base.art"); + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex.staged"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex.staged"); - int64_t aidl_return; - ASSERT_TRUE( - artd_ - ->cleanup( - { - PrimaryCurProfilePath{ - .userId = 1, .packageName = "com.android.foo", .profileName = "primary"}, - PrimaryCurProfilePath{ - .userId = 3, .packageName = "com.android.foo", .profileName = "primary"}, - }, - { - ArtifactsPath{.dexPath = "/system/app/Foo/Foo.apk", - .isa = "arm64", - .isInDalvikCache = true}, - ArtifactsPath{ - .dexPath = - android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/base.apk", - .isa = "arm64", - .isInDalvikCache = false}, - ArtifactsPath{.dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/2.apk", - .isa = "arm64", - .isInDalvikCache = false}, - }, - { - VdexPath{ArtifactsPath{ - .dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/1.apk", - .isa = "arm64", - .isInDalvikCache = false}}, - }, - { - RuntimeArtifactsPath{ - .packageName = "com.android.foo", .dexPath = "/a/b/base.apk", .isa = "arm64"}, - }, - &aidl_return) - .isOk()); + ASSERT_NO_FATAL_FAILURE(RunCleanup(/*keepPreRebootStagedFiles=*/true)); + Verify(); +} - for (const std::string& path : gc_removed_files) { - EXPECT_FALSE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be removed", path); - } +TEST_F(ArtdCleanupTest, cleanupRemovingPreRebootStagedFiles) { + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex.staged"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex.staged"); - for (const std::string& path : gc_kept_files) { - EXPECT_TRUE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be kept", path); - } + ASSERT_NO_FATAL_FAILURE(RunCleanup(/*keepPreRebootStagedFiles=*/false)); + Verify(); } TEST_F(ArtdTest, isInDalvikCache) { diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl index 4532a8fb11..54f55f8460 100644 --- a/artd/binder/com/android/server/art/IArtd.aidl +++ b/artd/binder/com/android/server/art/IArtd.aidl @@ -188,7 +188,8 @@ interface IArtd { long cleanup(in List profilesToKeep, in List artifactsToKeep, in List vdexFilesToKeep, - in List runtimeArtifactsToKeep); + in List runtimeArtifactsToKeep, + boolean keepPreRebootStagedFiles); /** * Returns whether the artifacts of the primary dex files should be in the global dalvik-cache diff --git a/artd/path_utils.cc b/artd/path_utils.cc index 7cd9413dea..5c6ad9dc02 100644 --- a/artd/path_utils.cc +++ b/artd/path_utils.cc @@ -23,6 +23,7 @@ #include "aidl/com/android/server/art/BnArtd.h" #include "android-base/errors.h" #include "android-base/result.h" +#include "android-base/strings.h" #include "arch/instruction_set.h" #include "base/file_utils.h" #include "base/macros.h" @@ -44,6 +45,7 @@ using ::aidl::com::android::server::art::OutputProfile; using ::aidl::com::android::server::art::ProfilePath; using ::aidl::com::android::server::art::RuntimeArtifactsPath; using ::aidl::com::android::server::art::VdexPath; +using ::android::base::EndsWith; using ::android::base::Error; using ::android::base::Result; using ::art::service::ValidateDexPath; @@ -111,11 +113,11 @@ std::vector ListManagedFiles(const std::string& android_data, // Profiles and artifacts for secondary dex files. Those files are in app data directories, so // we use more granular patterns to avoid accidentally deleting apps' files. std::string secondary_oat_dir = data_dir + "/**/oat"; - for (const char* maybe_tmp_suffix : {"", ".*.tmp"}) { - patterns.push_back(secondary_oat_dir + "/*.prof" + maybe_tmp_suffix); - patterns.push_back(secondary_oat_dir + "/*/*.odex" + maybe_tmp_suffix); - patterns.push_back(secondary_oat_dir + "/*/*.vdex" + maybe_tmp_suffix); - patterns.push_back(secondary_oat_dir + "/*/*.art" + maybe_tmp_suffix); + for (const char* suffix : {"", ".*.tmp", kPreRebootSuffix}) { + patterns.push_back(secondary_oat_dir + "/*.prof" + suffix); + patterns.push_back(secondary_oat_dir + "/*/*.odex" + suffix); + patterns.push_back(secondary_oat_dir + "/*/*.vdex" + suffix); + patterns.push_back(secondary_oat_dir + "/*/*.art" + suffix); } // Runtime image files. patterns.push_back(RuntimeImage::GetRuntimeImageDir(data_dir) + "**"); @@ -332,6 +334,10 @@ bool PreRebootFlag(const VdexPath& vdex_path) { return PreRebootFlag(vdex_path.get()); } +bool IsPreRebootStagedFile(std::string_view filename) { + return EndsWith(filename, kPreRebootSuffix); +} + void TestOnlySetListRootDir(std::string_view root_dir) { gListRootDir = root_dir; } } // namespace artd diff --git a/artd/path_utils.h b/artd/path_utils.h index 43a1043b42..1528d0610b 100644 --- a/artd/path_utils.h +++ b/artd/path_utils.h @@ -128,6 +128,8 @@ bool PreRebootFlag(const aidl::com::android::server::art::ArtifactsPath& artifac bool PreRebootFlag(const aidl::com::android::server::art::OutputArtifacts& artifacts); bool PreRebootFlag(const aidl::com::android::server::art::VdexPath& vdex_path); +bool IsPreRebootStagedFile(std::string_view filename); + // Sets the root dir for `ListManagedFiles` and `ListRuntimeImageFiles`. // The passed string must be alive until the test ends. // For testing use only. diff --git a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java index 727290f995..c4b5324e1a 100644 --- a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java +++ b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java @@ -1098,8 +1098,9 @@ public final class ArtManagerLocal { runtimeArtifactsToKeep.addAll(artifactLists.runtimeArtifacts()); } } - return mInjector.getArtd().cleanup( - profilesToKeep, artifactsToKeep, vdexFilesToKeep, runtimeArtifactsToKeep); + return mInjector.getArtd().cleanup(profilesToKeep, artifactsToKeep, vdexFilesToKeep, + runtimeArtifactsToKeep, + SdkLevel.isAtLeastV() && mInjector.getPreRebootDexoptJob().hasStarted()); } catch (RemoteException e) { Utils.logArtdException(e); return 0; diff --git a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java index 08ff12c0b2..099be02b13 100644 --- a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java +++ b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java @@ -71,6 +71,16 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { /** The slot that contains the OTA update, "_a" or "_b", or null for a Mainline update. */ @GuardedBy("this") @Nullable private String mOtaSlot = null; + /** + * Whether the job has started at least once, meaning the device is expected to have staged + * files, no matter it succeed, failed, or cancelled. + * + * Note that this flag is not persisted across system server restarts. It's possible that the + * value is lost due to a system server restart caused by a crash, but this is a minor case, so + * we don't handle it here for simplicity. + */ + @GuardedBy("this") private boolean mHasStarted = false; + public PreRebootDexoptJob(@NonNull Context context) { this(new Injector(context)); } @@ -164,6 +174,7 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { String otaSlot = mOtaSlot; var cancellationSignal = mCancellationSignal = new CancellationSignal(); + mHasStarted = true; mRunningJob = new CompletableFuture().runAsync(() -> { try { // TODO(b/336239721): Consume the result and report metrics. @@ -213,6 +224,10 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { } } + public synchronized boolean hasStarted() { + return mHasStarted; + } + /** * Injector pattern for testing purpose. * diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java index 098c28b108..1fd7e4bcfc 100644 --- a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java +++ b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java @@ -137,6 +137,7 @@ public class ArtManagerLocalTest { @Mock private ArtdRefCache.Pin mArtdPin; @Mock private DexMetadataHelper.Injector mDexMetadataHelperInjector; @Mock private Context mContext; + @Mock private PreRebootDexoptJob mPreRebootDexoptJob; private PackageState mPkgState1; private AndroidPackage mPkg1; private CheckedSecondaryDexInfo mPkg1SecondaryDexInfo1; @@ -180,6 +181,7 @@ public class ArtManagerLocalTest { .thenReturn(new ArtFileManager(mArtFileManagerInjector)); lenient().when(mInjector.getDexMetadataHelper()).thenReturn(mDexMetadataHelper); lenient().when(mInjector.getContext()).thenReturn(mContext); + lenient().when(mInjector.getPreRebootDexoptJob()).thenReturn(mPreRebootDexoptJob); lenient().when(mArtFileManagerInjector.getArtd()).thenReturn(mArtd); lenient().when(mArtFileManagerInjector.getUserManager()).thenReturn(mUserManager); @@ -1132,7 +1134,18 @@ public class ArtManagerLocalTest { } @Test - public void testCleanup() throws Exception { + public void testCleanupKeepPreRebootStagedFiles() throws Exception { + when(mPreRebootDexoptJob.hasStarted()).thenReturn(true); + testCleanup(true /* keepPreRebootStagedFiles */); + } + + @Test + public void testCleanupRemovePreRebootStagedFiles() throws Exception { + when(mPreRebootDexoptJob.hasStarted()).thenReturn(false); + testCleanup(false /* keepPreRebootStagedFiles */); + } + + private void testCleanup(boolean keepPreRebootStagedFiles) throws Exception { // It should keep all artifacts, but not runtime images. doReturn(createGetDexoptStatusResult( "speed-profile", "bg-dexopt", "location", ArtifactsLocation.NEXT_TO_DEX)) @@ -1190,7 +1203,8 @@ public class ArtManagerLocalTest { inAnyOrderDeepEquals(AidlUtils.buildRuntimeArtifactsPath( PKG_NAME_1, "/somewhere/app/foo/split_0.apk", "arm64"), AidlUtils.buildRuntimeArtifactsPath( - PKG_NAME_1, "/somewhere/app/foo/split_0.apk", "arm"))); + PKG_NAME_1, "/somewhere/app/foo/split_0.apk", "arm")), + eq(keepPreRebootStagedFiles)); } @Test diff --git a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java index bc30d2c058..ba4e7bab52 100644 --- a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java +++ b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java @@ -150,7 +150,11 @@ public class PreRebootDexoptJobTest { public void testStart() { when(mPreRebootDriver.run(any(), any())).thenReturn(true); - Utils.getFuture(mPreRebootDexoptJob.start()); + assertThat(mPreRebootDexoptJob.hasStarted()).isFalse(); + Future future = mPreRebootDexoptJob.start(); + assertThat(mPreRebootDexoptJob.hasStarted()).isTrue(); + + Utils.getFuture(future); } @Test -- cgit v1.2.3