ART services: Change artd to support cancellation.
Bug: 244412198
Test: m test-art-host-gtest-art_artd_tests
Ignore-AOSP-First: ART Services.
Change-Id: I8b750bf030677d3501c0ee104a63793cfa84742f
diff --git a/artd/artd.cc b/artd/artd.cc
index db317f9..0db2efa 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -22,6 +22,7 @@
#include <unistd.h>
#include <climits>
+#include <csignal>
#include <cstdint>
#include <cstring>
#include <filesystem>
@@ -40,6 +41,7 @@
#include "aidl/com/android/server/art/BnArtd.h"
#include "aidl/com/android/server/art/DexoptTrigger.h"
+#include "aidl/com/android/server/art/IArtdCancellationSignal.h"
#include "android-base/errors.h"
#include "android-base/file.h"
#include "android-base/logging.h"
@@ -47,6 +49,7 @@
#include "android-base/scopeguard.h"
#include "android-base/strings.h"
#include "android/binder_auto_utils.h"
+#include "android/binder_interface_utils.h"
#include "android/binder_manager.h"
#include "android/binder_process.h"
#include "base/compiler_filter.h"
@@ -77,6 +80,7 @@
using ::aidl::com::android::server::art::FsPermission;
using ::aidl::com::android::server::art::GetDexoptNeededResult;
using ::aidl::com::android::server::art::GetOptimizationStatusResult;
+using ::aidl::com::android::server::art::IArtdCancellationSignal;
using ::aidl::com::android::server::art::OutputArtifacts;
using ::aidl::com::android::server::art::OutputProfile;
using ::aidl::com::android::server::art::PriorityClass;
@@ -100,6 +104,7 @@
using TmpRefProfilePath = ProfilePath::TmpRefProfilePath;
constexpr const char* kServiceName = "artd";
+constexpr const char* kArtdCancellationSignalType = "ArtdCancellationSignal";
// Timeout for short operations, such as merging profiles.
constexpr int kShortTimeoutSec = 60; // 1 minute.
@@ -269,6 +274,21 @@
FileVisibility::NOT_OTHER_READABLE;
}
+Result<ArtdCancellationSignal*> ToArtdCancellationSignal(IArtdCancellationSignal* input) {
+ if (input == nullptr) {
+ return Error() << "Cancellation signal must not be nullptr";
+ }
+ // We cannot use `dynamic_cast` because ART code is compiled with `-fno-rtti`, so we have to check
+ // the magic number.
+ int64_t type;
+ if (!input->getType(&type).isOk() ||
+ type != reinterpret_cast<intptr_t>(kArtdCancellationSignalType)) {
+ // The cancellation signal must be created by `Artd::createCancellationSignal`.
+ return Error() << "Invalid cancellation signal type";
+ }
+ return static_cast<ArtdCancellationSignal*>(input);
+}
+
class FdLogger {
public:
void Add(const NewFile& file) { fd_mapping_.emplace_back(file.Fd(), file.TempPath()); }
@@ -559,16 +579,18 @@
return ScopedAStatus::ok();
}
-ndk::ScopedAStatus Artd::dexopt(const OutputArtifacts& in_outputArtifacts,
- const std::string& in_dexFile,
- const std::string& in_instructionSet,
- const std::string& in_classLoaderContext,
- const std::string& in_compilerFilter,
- const std::optional<ProfilePath>& in_profile,
- const std::optional<VdexPath>& in_inputVdex,
- PriorityClass in_priorityClass,
- const DexoptOptions& in_dexoptOptions,
- DexoptResult* _aidl_return) {
+ndk::ScopedAStatus Artd::dexopt(
+ const OutputArtifacts& in_outputArtifacts,
+ const std::string& in_dexFile,
+ const std::string& in_instructionSet,
+ const std::string& in_classLoaderContext,
+ const std::string& in_compilerFilter,
+ const std::optional<ProfilePath>& in_profile,
+ const std::optional<VdexPath>& in_inputVdex,
+ PriorityClass in_priorityClass,
+ const DexoptOptions& in_dexoptOptions,
+ const std::shared_ptr<IArtdCancellationSignal>& in_cancellationSignal,
+ DexoptResult* _aidl_return) {
_aidl_return->cancelled = false;
std::string oat_path = OR_RETURN_FATAL(BuildOatPath(in_outputArtifacts.artifactsPath));
@@ -579,6 +601,8 @@
in_profile.has_value() ?
std::make_optional(OR_RETURN_FATAL(BuildProfileOrDmPath(in_profile.value()))) :
std::nullopt;
+ ArtdCancellationSignal* cancellation_signal =
+ OR_RETURN_FATAL(ToArtdCancellationSignal(in_cancellationSignal.get()));
std::unique_ptr<ClassLoaderContext> context =
ClassLoaderContext::Create(in_classLoaderContext.c_str());
@@ -677,12 +701,37 @@
LOG(INFO) << "Running dex2oat: " << Join(args.Get(), /*separator=*/" ")
<< "\nOpened FDs: " << fd_logger;
+ ExecCallbacks callbacks{
+ .on_start =
+ [&](pid_t pid) {
+ std::lock_guard<std::mutex> lock(cancellation_signal->mu_);
+ cancellation_signal->pids_.insert(pid);
+ // Handle cancellation signals sent before the process starts.
+ if (cancellation_signal->is_cancelled_) {
+ int res = kill_(pid, SIGKILL);
+ DCHECK_EQ(res, 0);
+ }
+ },
+ .on_end =
+ [&](pid_t pid) {
+ std::lock_guard<std::mutex> lock(cancellation_signal->mu_);
+ // The pid should no longer receive kill signals sent by `cancellation_signal`.
+ cancellation_signal->pids_.erase(pid);
+ },
+ };
+
ProcessStat stat;
- Result<int> result = ExecAndReturnCode(args.Get(), kLongTimeoutSec, &stat);
+ Result<int> result = ExecAndReturnCode(args.Get(), kLongTimeoutSec, callbacks, &stat);
_aidl_return->wallTimeMs = stat.wall_time_ms;
_aidl_return->cpuTimeMs = stat.cpu_time_ms;
if (!result.ok()) {
- // TODO(b/244412198): Return cancelled=true if dexopt is cancelled upon request.
+ {
+ std::lock_guard<std::mutex> lock(cancellation_signal->mu_);
+ if (cancellation_signal->is_cancelled_) {
+ _aidl_return->cancelled = true;
+ return ScopedAStatus::ok();
+ }
+ }
return NonFatal("Failed to run dex2oat: " + result.error().message());
}
if (result.value() != 0) {
@@ -694,6 +743,27 @@
return ScopedAStatus::ok();
}
+ScopedAStatus ArtdCancellationSignal::cancel() {
+ std::lock_guard<std::mutex> lock(mu_);
+ is_cancelled_ = true;
+ for (pid_t pid : pids_) {
+ int res = kill_(pid, SIGKILL);
+ DCHECK_EQ(res, 0);
+ }
+ return ScopedAStatus::ok();
+}
+
+ScopedAStatus ArtdCancellationSignal::getType(int64_t* _aidl_return) {
+ *_aidl_return = reinterpret_cast<intptr_t>(kArtdCancellationSignalType);
+ return ScopedAStatus::ok();
+}
+
+ScopedAStatus Artd::createCancellationSignal(
+ std::shared_ptr<IArtdCancellationSignal>* _aidl_return) {
+ *_aidl_return = ndk::SharedRefBase::make<ArtdCancellationSignal>(kill_);
+ return ScopedAStatus::ok();
+}
+
Result<void> Artd::Start() {
ScopedAStatus status = ScopedAStatus::fromStatus(
AServiceManager_registerLazyService(this->asBinder().get(), kServiceName));
@@ -870,10 +940,11 @@
android::base::Result<int> Artd::ExecAndReturnCode(const std::vector<std::string>& args,
int timeout_sec,
+ const ExecCallbacks& callbacks,
ProcessStat* stat) const {
std::string error_msg;
ExecResult result =
- exec_utils_->ExecAndReturnResult(args, timeout_sec, ExecCallbacks(), stat, &error_msg);
+ exec_utils_->ExecAndReturnResult(args, timeout_sec, callbacks, stat, &error_msg);
if (result.status != ExecResult::kExited) {
return Error() << error_msg;
}
diff --git a/artd/artd.h b/artd/artd.h
index 931f606..d2952a2 100644
--- a/artd/artd.h
+++ b/artd/artd.h
@@ -17,14 +17,21 @@
#ifndef ART_ARTD_ARTD_H_
#define ART_ARTD_ARTD_H_
+#include <sys/types.h>
+
+#include <csignal>
#include <cstdint>
+#include <functional>
#include <memory>
#include <mutex>
#include <string>
+#include <unordered_map>
+#include <unordered_set>
#include <utility>
#include <vector>
#include "aidl/com/android/server/art/BnArtd.h"
+#include "aidl/com/android/server/art/BnArtdCancellationSignal.h"
#include "android-base/result.h"
#include "android-base/thread_annotations.h"
#include "android/binder_auto_utils.h"
@@ -36,12 +43,34 @@
namespace art {
namespace artd {
+class ArtdCancellationSignal : public aidl::com::android::server::art::BnArtdCancellationSignal {
+ public:
+ explicit ArtdCancellationSignal(std::function<int(pid_t, int)> kill_func)
+ : kill_(std::move(kill_func)) {}
+
+ ndk::ScopedAStatus cancel() override;
+
+ ndk::ScopedAStatus getType(int64_t* _aidl_return) override;
+
+ private:
+ std::mutex mu_;
+ // True if cancellation has been signaled.
+ bool is_cancelled_ GUARDED_BY(mu_) = false;
+ // The pids of currently running child processes that are bound to this signal.
+ std::unordered_set<pid_t> pids_ GUARDED_BY(mu_);
+
+ std::function<int(pid_t, int)> kill_;
+
+ friend class Artd;
+};
+
class Artd : public aidl::com::android::server::art::BnArtd {
public:
explicit Artd(std::unique_ptr<art::tools::SystemProperties> props =
std::make_unique<art::tools::SystemProperties>(),
- std::unique_ptr<ExecUtils> exec_utils = std::make_unique<ExecUtils>())
- : props_(std::move(props)), exec_utils_(std::move(exec_utils)) {}
+ std::unique_ptr<ExecUtils> exec_utils = std::make_unique<ExecUtils>(),
+ std::function<int(pid_t, int)> kill_func = kill)
+ : props_(std::move(props)), exec_utils_(std::move(exec_utils)), kill_(std::move(kill_func)) {}
ndk::ScopedAStatus isAlive(bool* _aidl_return) override;
@@ -100,8 +129,14 @@
const std::optional<aidl::com::android::server::art::VdexPath>& in_inputVdex,
aidl::com::android::server::art::PriorityClass in_priorityClass,
const aidl::com::android::server::art::DexoptOptions& in_dexoptOptions,
+ const std::shared_ptr<aidl::com::android::server::art::IArtdCancellationSignal>&
+ in_cancellationSignal,
aidl::com::android::server::art::DexoptResult* _aidl_return) override;
+ ndk::ScopedAStatus createCancellationSignal(
+ std::shared_ptr<aidl::com::android::server::art::IArtdCancellationSignal>* _aidl_return)
+ override;
+
android::base::Result<void> Start();
private:
@@ -118,6 +153,7 @@
android::base::Result<int> ExecAndReturnCode(const std::vector<std::string>& arg_vector,
int timeout_sec,
+ const ExecCallbacks& callbacks = ExecCallbacks(),
ProcessStat* stat = nullptr) const;
android::base::Result<std::string> GetProfman();
@@ -150,6 +186,7 @@
std::unique_ptr<art::tools::SystemProperties> props_;
std::unique_ptr<ExecUtils> exec_utils_;
+ std::function<int(pid_t, int)> kill_;
};
} // namespace artd
diff --git a/artd/artd_test.cc b/artd/artd_test.cc
index 92180b1..3d2fbf3 100644
--- a/artd/artd_test.cc
+++ b/artd/artd_test.cc
@@ -16,12 +16,19 @@
#include "artd.h"
+#include <sys/types.h>
+
#include <algorithm>
+#include <chrono>
+#include <condition_variable>
+#include <csignal>
#include <filesystem>
#include <functional>
#include <memory>
+#include <mutex>
#include <optional>
#include <string>
+#include <thread>
#include <type_traits>
#include <vector>
@@ -54,11 +61,13 @@
using ::aidl::com::android::server::art::DexoptResult;
using ::aidl::com::android::server::art::FileVisibility;
using ::aidl::com::android::server::art::FsPermission;
+using ::aidl::com::android::server::art::IArtdCancellationSignal;
using ::aidl::com::android::server::art::OutputArtifacts;
using ::aidl::com::android::server::art::OutputProfile;
using ::aidl::com::android::server::art::PriorityClass;
using ::aidl::com::android::server::art::ProfilePath;
using ::aidl::com::android::server::art::VdexPath;
+using ::android::base::Error;
using ::android::base::make_scope_guard;
using ::android::base::ParseInt;
using ::android::base::ReadFileToString;
@@ -174,10 +183,10 @@
// to a conflict between gmock and android-base/logging.h (b/132668253).
ExecResult ExecAndReturnResult(const std::vector<std::string>& arg_vector,
int,
- const ExecCallbacks&,
+ const ExecCallbacks& callbacks,
ProcessStat* stat,
std::string*) const override {
- Result<int> code = DoExecAndReturnCode(arg_vector, stat);
+ Result<int> code = DoExecAndReturnCode(arg_vector, callbacks, stat);
if (code.ok()) {
return {.status = ExecResult::kExited, .exit_code = code.value()};
}
@@ -186,7 +195,9 @@
MOCK_METHOD(Result<int>,
DoExecAndReturnCode,
- (const std::vector<std::string>& arg_vector, ProcessStat* stat),
+ (const std::vector<std::string>& arg_vector,
+ const ExecCallbacks& callbacks,
+ ProcessStat* stat),
(const));
};
@@ -199,7 +210,8 @@
EXPECT_CALL(*mock_props_, GetProperty).Times(AnyNumber()).WillRepeatedly(Return(""));
auto mock_exec_utils = std::make_unique<MockExecUtils>();
mock_exec_utils_ = mock_exec_utils.get();
- artd_ = ndk::SharedRefBase::make<Artd>(std::move(mock_props), std::move(mock_exec_utils));
+ artd_ = ndk::SharedRefBase::make<Artd>(
+ std::move(mock_props), std::move(mock_exec_utils), mock_kill_.AsStdFunction());
scratch_dir_ = std::make_unique<ScratchDir>();
scratch_path_ = scratch_dir_->GetPath();
// Remove the trailing '/';
@@ -259,9 +271,12 @@
}
void RunDexopt(binder_exception_t expected_status = EX_NONE,
- Matcher<DexoptResult> aidl_return_matcher = Field(&DexoptResult::cancelled,
- false)) {
+ Matcher<DexoptResult> aidl_return_matcher = Field(&DexoptResult::cancelled, false),
+ std::shared_ptr<IArtdCancellationSignal> cancellation_signal = nullptr) {
InitDexoptInputFiles();
+ if (cancellation_signal == nullptr) {
+ ASSERT_TRUE(artd_->createCancellationSignal(&cancellation_signal).isOk());
+ }
DexoptResult aidl_return;
ndk::ScopedAStatus status = artd_->dexopt(output_artifacts_,
dex_file_,
@@ -272,6 +287,7 @@
vdex_path_,
priority_class_,
dexopt_options_,
+ cancellation_signal,
&aidl_return);
ASSERT_EQ(status.getExceptionCode(), expected_status) << status.getMessage();
if (status.isOk()) {
@@ -295,6 +311,7 @@
ScopedUnsetEnvironmentVariable android_data_env_ = ScopedUnsetEnvironmentVariable("ANDROID_DATA");
MockSystemProperties* mock_props_;
MockExecUtils* mock_exec_utils_;
+ MockFunction<int(pid_t, int)> mock_kill_;
std::string dex_file_;
std::string isa_;
@@ -440,10 +457,11 @@
Flag("--profile-file-fd=",
FdOf(android_data_ +
"/misc/profiles/ref/com.android.foo/primary.prof.12345.tmp"))))),
+ _,
_))
.WillOnce(DoAll(WithArg<0>(WriteToFdFlag("--oat-fd=", "oat")),
WithArg<0>(WriteToFdFlag("--output-vdex-fd=", "vdex")),
- SetArgPointee<1>(ProcessStat{.wall_time_ms = 100, .cpu_time_ms = 400}),
+ SetArgPointee<2>(ProcessStat{.wall_time_ms = 100, .cpu_time_ms = 400}),
Return(0)));
RunDexopt(EX_NONE,
AllOf(Field(&DexoptResult::cancelled, false),
@@ -464,6 +482,7 @@
ElementsAre(FdOf(clc_1_), FdOf(clc_2_)))),
Contains(Flag("--class-loader-context=", class_loader_context_)),
Contains(Flag("--classpath-dir=", scratch_path_ + "/a")))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -475,6 +494,7 @@
_,
AllOf(Not(Contains(Flag("--dm-fd=", _))),
Not(Contains(Flag("--input-vdex-fd=", _))))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -489,6 +509,7 @@
AllOf(Not(Contains(Flag("--dm-fd=", _))),
Contains(Flag("--input-vdex-fd=",
FdOf(scratch_path_ + "/a/oat/arm64/b.vdex"))))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -502,6 +523,7 @@
_,
AllOf(Contains(Flag("--dm-fd=", FdOf(scratch_path_ + "/a/b.dm"))),
Not(Contains(Flag("--input-vdex-fd=", _))))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -514,6 +536,7 @@
AllOf(Not(Contains(Flag("--set-task-profile=", _))),
Not(Contains(Flag("--set-priority=", _)))),
Contains(Flag("--compact-dex-level=", "none"))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -527,6 +550,7 @@
AllOf(Contains(Flag("--set-task-profile=", "Dex2OatBootComplete")),
Contains(Flag("--set-priority=", "background"))),
Contains(Flag("--compact-dex-level=", "none"))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -540,6 +564,7 @@
AllOf(Contains(Flag("--set-task-profile=", "Dex2OatBootComplete")),
Contains(Flag("--set-priority=", "background"))),
Contains(Flag("--compact-dex-level=", "none"))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -553,6 +578,7 @@
AllOf(Contains(Flag("--set-task-profile=", "Dex2OatBootComplete")),
Contains(Flag("--set-priority=", "background"))),
Not(Contains(Flag("--compact-dex-level=", _)))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -576,6 +602,7 @@
Not(Contains("--debuggable")),
Not(Contains(Flag("--app-image-fd=", _))),
Not(Contains(Flag("-Xhidden-api-policy:", _))))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -598,6 +625,7 @@
Contains(Flag("-Xtarget-sdk-version:", "456")),
Contains("--debuggable"),
Contains(Flag("-Xhidden-api-policy:", "enabled")))),
+ _,
_))
.WillOnce(DoAll(WithArg<0>(WriteToFdFlag("--app-image-fd=", "art")), Return(0)));
RunDexopt();
@@ -624,6 +652,7 @@
Not(Contains(Flag("-Xms", _))),
Not(Contains(Flag("-Xmx", _))),
Not(Contains("--compile-individually")))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -663,6 +692,7 @@
Contains(Flag("-Xms", "xms")),
Contains(Flag("-Xmx", "xmx")),
Contains("--compile-individually"))),
+ _,
_))
.WillOnce(Return(0));
RunDexopt();
@@ -682,6 +712,7 @@
DoExecAndReturnCode(
WhenSplitBy(
"--", _, AllOf(Not(Contains(Flag("--cpu-set=", _))), Contains(Not(Flag("-j", _))))),
+ _,
_))
.WillOnce(Return(0));
priority_class_ = PriorityClass::BOOT;
@@ -696,6 +727,7 @@
DoExecAndReturnCode(
WhenSplitBy(
"--", _, AllOf(Contains(Flag("--cpu-set=", "0,2")), Contains(Flag("-j", "4")))),
+ _,
_))
.Times(3)
.WillRepeatedly(Return(0));
@@ -732,6 +764,7 @@
DoExecAndReturnCode(
WhenSplitBy(
"--", _, AllOf(Contains(Flag("--cpu-set=", "0,1,2,3")), Contains(Flag("-j", "8")))),
+ _,
_))
.WillOnce(Return(0));
priority_class_ = PriorityClass::BOOT;
@@ -746,6 +779,7 @@
DoExecAndReturnCode(
WhenSplitBy(
"--", _, AllOf(Contains(Flag("--cpu-set=", "0,2,3")), Contains(Flag("-j", "6")))),
+ _,
_))
.WillOnce(Return(0));
priority_class_ = PriorityClass::INTERACTIVE_FAST;
@@ -761,6 +795,7 @@
DoExecAndReturnCode(
WhenSplitBy(
"--", _, AllOf(Contains(Flag("--cpu-set=", "0,2")), Contains(Flag("-j", "4")))),
+ _,
_))
.WillOnce(Return(0));
priority_class_ = PriorityClass::INTERACTIVE;
@@ -774,6 +809,7 @@
*mock_exec_utils_,
DoExecAndReturnCode(
WhenSplitBy("--", _, AllOf(Contains(Flag("--cpu-set=", "0")), Contains(Flag("-j", "2")))),
+ _,
_))
.WillOnce(Return(0));
priority_class_ = PriorityClass::BACKGROUND;
@@ -782,7 +818,7 @@
TEST_F(ArtdTest, dexoptFailed) {
dexopt_options_.generateAppImage = true;
- EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _))
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
.WillOnce(DoAll(WithArg<0>(WriteToFdFlag("--oat-fd=", "oat")),
WithArg<0>(WriteToFdFlag("--output-vdex-fd=", "vdex")),
WithArg<0>(WriteToFdFlag("--app-image-fd=", "art")),
@@ -794,6 +830,97 @@
EXPECT_FALSE(std::filesystem::exists(scratch_path_ + "/a/oat/arm64/b.art"));
}
+TEST_F(ArtdTest, dexoptCancelledBeforeDex2oat) {
+ std::shared_ptr<IArtdCancellationSignal> cancellation_signal;
+ ASSERT_TRUE(artd_->createCancellationSignal(&cancellation_signal).isOk());
+
+ constexpr pid_t kPid = 123;
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce([&](auto, const ExecCallbacks& callbacks, auto) {
+ callbacks.on_start(kPid);
+ callbacks.on_end(kPid);
+ return Error();
+ });
+ EXPECT_CALL(mock_kill_, Call(kPid, SIGKILL));
+
+ cancellation_signal->cancel();
+
+ RunDexopt(EX_NONE, Field(&DexoptResult::cancelled, true), cancellation_signal);
+
+ EXPECT_FALSE(std::filesystem::exists(scratch_path_ + "/a/oat/arm64/b.odex"));
+ EXPECT_FALSE(std::filesystem::exists(scratch_path_ + "/a/oat/arm64/b.vdex"));
+}
+
+TEST_F(ArtdTest, dexoptCancelledDuringDex2oat) {
+ std::shared_ptr<IArtdCancellationSignal> cancellation_signal;
+ ASSERT_TRUE(artd_->createCancellationSignal(&cancellation_signal).isOk());
+
+ constexpr pid_t kPid = 123;
+ constexpr std::chrono::duration<int> kTimeout = std::chrono::seconds(1);
+
+ std::condition_variable process_started_cv, process_killed_cv;
+ std::mutex mu;
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce([&](auto, const ExecCallbacks& callbacks, auto) {
+ std::unique_lock<std::mutex> lock(mu);
+ // Step 2.
+ callbacks.on_start(kPid);
+ process_started_cv.notify_one();
+ EXPECT_EQ(process_killed_cv.wait_for(lock, kTimeout), std::cv_status::no_timeout);
+ // Step 5.
+ callbacks.on_end(kPid);
+ return Error();
+ });
+
+ EXPECT_CALL(mock_kill_, Call(kPid, SIGKILL)).WillOnce([&](auto, auto) {
+ // Step 4.
+ process_killed_cv.notify_one();
+ return 0;
+ });
+
+ std::thread t;
+ {
+ std::unique_lock<std::mutex> lock(mu);
+ // Step 1.
+ t = std::thread(
+ [&] { RunDexopt(EX_NONE, Field(&DexoptResult::cancelled, true), cancellation_signal); });
+ EXPECT_EQ(process_started_cv.wait_for(lock, kTimeout), std::cv_status::no_timeout);
+ // Step 3.
+ cancellation_signal->cancel();
+ }
+
+ t.join();
+
+ // Step 6.
+ EXPECT_FALSE(std::filesystem::exists(scratch_path_ + "/a/oat/arm64/b.odex"));
+ EXPECT_FALSE(std::filesystem::exists(scratch_path_ + "/a/oat/arm64/b.vdex"));
+}
+
+TEST_F(ArtdTest, dexoptCancelledAfterDex2oat) {
+ std::shared_ptr<IArtdCancellationSignal> cancellation_signal;
+ ASSERT_TRUE(artd_->createCancellationSignal(&cancellation_signal).isOk());
+
+ constexpr pid_t kPid = 123;
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce([&](auto, const ExecCallbacks& callbacks, auto) {
+ callbacks.on_start(kPid);
+ callbacks.on_end(kPid);
+ return 0;
+ });
+ EXPECT_CALL(mock_kill_, Call).Times(0);
+
+ RunDexopt(EX_NONE, Field(&DexoptResult::cancelled, false), cancellation_signal);
+
+ // This signal should be ignored.
+ cancellation_signal->cancel();
+
+ EXPECT_TRUE(std::filesystem::exists(scratch_path_ + "/a/oat/arm64/b.odex"));
+ EXPECT_TRUE(std::filesystem::exists(scratch_path_ + "/a/oat/arm64/b.vdex"));
+}
+
TEST_F(ArtdTest, isProfileUsable) {
std::string profile_file = OR_FATAL(BuildProfileOrDmPath(profile_path_.value()));
CreateFile(profile_file);
@@ -807,6 +934,7 @@
AllOf(Contains(art_root_ + "/bin/profman"),
Contains(Flag("--reference-profile-file-fd=", FdOf(profile_file))),
Contains(Flag("--apk-fd=", FdOf(dex_file_))))),
+ _,
_))
.WillOnce(Return(ProfmanResult::kSkipCompilationSmallDelta));
@@ -820,7 +948,7 @@
CreateFile(profile_file);
CreateFile(dex_file_);
- EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _))
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
.WillOnce(Return(ProfmanResult::kSkipCompilationEmptyProfiles));
bool result;
@@ -841,7 +969,7 @@
CreateFile(profile_file);
CreateFile(dex_file_);
- EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _)).WillOnce(Return(100));
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _)).WillOnce(Return(100));
bool result;
ndk::ScopedAStatus status = artd_->isProfileUsable(profile_path_.value(), dex_file_, &result);
@@ -895,6 +1023,7 @@
Contains("--copy-and-update-profile-key"),
Contains(Flag("--profile-file-fd=", FdOf(src_file))),
Contains(Flag("--apk-fd=", FdOf(dex_file_))))),
+ _,
_))
.WillOnce(DoAll(WithArg<0>(WriteToFdFlag("--reference-profile-file-fd=", "def")),
Return(ProfmanResult::kCopyAndUpdateSuccess)));
@@ -915,7 +1044,7 @@
CreateFile(dex_file_);
- EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _))
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
.WillOnce(Return(ProfmanResult::kCopyAndUpdateNoUpdate));
bool result;
@@ -944,7 +1073,7 @@
CreateFile(dex_file_);
- EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _)).WillOnce(Return(100));
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _)).WillOnce(Return(100));
bool result;
ndk::ScopedAStatus status = artd_->copyAndRewriteProfile(src, &dst, dex_file_, &result);
diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl
index a3eab2f..04ab0b5 100644
--- a/artd/binder/com/android/server/art/IArtd.aidl
+++ b/artd/binder/com/android/server/art/IArtd.aidl
@@ -122,5 +122,11 @@
in @nullable com.android.server.art.ProfilePath profile,
in @nullable com.android.server.art.VdexPath inputVdex,
com.android.server.art.PriorityClass priorityClass,
- in com.android.server.art.DexoptOptions dexoptOptions);
+ in com.android.server.art.DexoptOptions dexoptOptions,
+ in com.android.server.art.IArtdCancellationSignal cancellationSignal);
+
+ /**
+ * Returns a cancellation signal which can be used to cancel {@code dexopt} calls.
+ */
+ com.android.server.art.IArtdCancellationSignal createCancellationSignal();
}
diff --git a/artd/binder/com/android/server/art/IArtdCancellationSignal.aidl b/artd/binder/com/android/server/art/IArtdCancellationSignal.aidl
new file mode 100644
index 0000000..fb15e64
--- /dev/null
+++ b/artd/binder/com/android/server/art/IArtdCancellationSignal.aidl
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.art;
+
+/**
+ * Similar to `android.os.CancellationSignal` but for artd. Must be created by
+ * `IArtd.createCancellationSignal`.
+ *
+ * @hide
+ */
+interface IArtdCancellationSignal {
+ oneway void cancel();
+
+ /** For artd internal type-checking. DO NOT USE. */
+ long getType();
+}
diff --git a/libartservice/service/java/com/android/server/art/PrimaryDexOptimizer.java b/libartservice/service/java/com/android/server/art/PrimaryDexOptimizer.java
index b2875e8..74ebf7d 100644
--- a/libartservice/service/java/com/android/server/art/PrimaryDexOptimizer.java
+++ b/libartservice/service/java/com/android/server/art/PrimaryDexOptimizer.java
@@ -170,9 +170,12 @@
? ProfilePath.tmpRefProfilePath(profile.profilePath)
: null;
+ IArtdCancellationSignal artdCancellationSignal =
+ mInjector.getArtd().createCancellationSignal();
+
DexoptResult dexoptResult = dexoptFile(target, inputProfile,
getDexoptNeededResult, permissionSettings,
- params.getPriorityClass(), dexoptOptions);
+ params.getPriorityClass(), dexoptOptions, artdCancellationSignal);
status = dexoptResult.cancelled ? OptimizeResult.OPTIMIZE_CANCELLED
: OptimizeResult.OPTIMIZE_PERFORMED;
wallTimeMs = dexoptResult.wallTimeMs;
@@ -441,7 +444,8 @@
private DexoptResult dexoptFile(@NonNull DexoptTarget target, @Nullable ProfilePath profile,
@NonNull GetDexoptNeededResult getDexoptNeededResult,
@NonNull PermissionSettings permissionSettings, @PriorityClass int priorityClass,
- @NonNull DexoptOptions dexoptOptions) throws RemoteException {
+ @NonNull DexoptOptions dexoptOptions, IArtdCancellationSignal artdCancellationSignal)
+ throws RemoteException {
OutputArtifacts outputArtifacts = AidlUtils.buildOutputArtifacts(target.dexInfo().dexPath(),
target.isa(), target.isInDalvikCache(), permissionSettings);
@@ -450,7 +454,7 @@
return mInjector.getArtd().dexopt(outputArtifacts, target.dexInfo().dexPath(), target.isa(),
target.dexInfo().classLoaderContext(), target.compilerFilter(), profile, inputVdex,
- priorityClass, dexoptOptions);
+ priorityClass, dexoptOptions, artdCancellationSignal);
}
@Nullable
diff --git a/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerParameterizedTest.java b/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerParameterizedTest.java
index 3de422f..a6f5967 100644
--- a/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerParameterizedTest.java
+++ b/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerParameterizedTest.java
@@ -32,7 +32,9 @@
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.isNull;
import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
import android.os.Process;
import android.os.ServiceSpecificException;
@@ -201,6 +203,8 @@
dexoptOptions.generateAppImage = false;
dexoptOptions.hiddenApiPolicyEnabled = mParams.mExpectedIsHiddenApiPolicyEnabled;
+ when(mArtd.createCancellationSignal()).thenReturn(mock(IArtdCancellationSignal.class));
+
// The first one is normal.
doReturn(dexoptIsNeeded())
.when(mArtd)
@@ -214,7 +218,7 @@
eq("/data/app/foo/base.apk"), eq("arm64"), eq("PCL[]"),
eq(mParams.mExpectedCompilerFilter), isNull() /* profile */,
isNull() /* inputVdex */, eq(PriorityClass.INTERACTIVE),
- deepEq(dexoptOptions));
+ deepEq(dexoptOptions), any());
// The second one fails on `dexopt`.
doReturn(dexoptIsNeeded())
@@ -228,7 +232,7 @@
eq("/data/app/foo/base.apk"), eq("arm"), eq("PCL[]"),
eq(mParams.mExpectedCompilerFilter), isNull() /* profile */,
isNull() /* inputVdex */, eq(PriorityClass.INTERACTIVE),
- deepEq(dexoptOptions));
+ deepEq(dexoptOptions), any());
// The third one doesn't need dexopt.
doReturn(dexoptIsNotNeeded())
@@ -249,7 +253,7 @@
eq("/data/app/foo/split_0.apk"), eq("arm"), eq("PCL[base.apk]"),
eq(mParams.mExpectedCompilerFilter), isNull() /* profile */,
isNull() /* inputVdex */, eq(PriorityClass.INTERACTIVE),
- deepEq(dexoptOptions));
+ deepEq(dexoptOptions), any());
assertThat(mPrimaryDexOptimizer.dexopt(mPkgState, mPkg, mOptimizeParams))
.comparingElementsUsing(TestingUtils.<DexFileOptimizeResult>deepEquality())
diff --git a/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java b/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java
index 498657f..51ff455 100644
--- a/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java
+++ b/libartservice/service/javatests/com/android/server/art/PrimaryDexOptimizerTest.java
@@ -29,6 +29,7 @@
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.isNull;
import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -95,9 +96,13 @@
.thenReturn(dexoptIsNeeded());
lenient()
.when(mArtd.dexopt(
- any(), any(), any(), any(), any(), any(), any(), anyInt(), any()))
+ any(), any(), any(), any(), any(), any(), any(), anyInt(), any(), any()))
.thenReturn(mDexoptResult);
+ lenient()
+ .when(mArtd.createCancellationSignal())
+ .thenReturn(mock(IArtdCancellationSignal.class));
+
mUsedProfiles = new ArrayList<>();
}
@@ -110,7 +115,7 @@
doReturn(mDexoptResult)
.when(mArtd)
.dexopt(any(), eq(mDexPath), eq("arm64"), any(), any(), any(), isNull(), anyInt(),
- any());
+ any(), any());
// ArtifactsPath, isInDalvikCache=true.
doReturn(dexoptIsNeeded(ArtifactsLocation.DALVIK_CACHE))
@@ -121,7 +126,7 @@
.dexopt(any(), eq(mDexPath), eq("arm"), any(), any(), any(),
deepEq(VdexPath.artifactsPath(AidlUtils.buildArtifactsPath(
mDexPath, "arm", true /* isInDalvikCache */))),
- anyInt(), any());
+ anyInt(), any(), any());
// ArtifactsPath, isInDalvikCache=false.
doReturn(dexoptIsNeeded(ArtifactsLocation.NEXT_TO_DEX))
@@ -132,7 +137,7 @@
.dexopt(any(), eq(mSplit0DexPath), eq("arm64"), any(), any(), any(),
deepEq(VdexPath.artifactsPath(AidlUtils.buildArtifactsPath(
mSplit0DexPath, "arm64", false /* isInDalvikCache */))),
- anyInt(), any());
+ anyInt(), any(), any());
// DexMetadataPath.
doReturn(dexoptIsNeeded(ArtifactsLocation.DM))
@@ -143,7 +148,7 @@
.dexopt(any(), eq(mSplit0DexPath), eq("arm"), any(), any(), any(),
deepEq(VdexPath.dexMetadataPath(
AidlUtils.buildDexMetadataPath(mSplit0DexPath))),
- anyInt(), any());
+ anyInt(), any(), any());
mPrimaryDexOptimizer.dexopt(mPkgState, mPkg, mOptimizeParams);
}
@@ -253,7 +258,8 @@
when(mArtd.getProfileVisibility(deepEq(mRefProfile)))
.thenReturn(FileVisibility.NOT_OTHER_READABLE);
- when(mArtd.dexopt(any(), eq(mDexPath), any(), any(), any(), any(), any(), anyInt(), any()))
+ when(mArtd.dexopt(any(), eq(mDexPath), any(), any(), any(), any(), any(), anyInt(), any(),
+ any()))
.thenThrow(ServiceSpecificException.class);
mPrimaryDexOptimizer.dexopt(mPkgState, mPkg, mOptimizeParams);
@@ -347,7 +353,7 @@
-> artifacts.permissionSettings.fileFsPermission.isOtherReadable == true),
eq(dexPath), eq(isa), any(), eq("speed-profile"),
deepEq(ProfilePath.tmpRefProfilePath(profile.profilePath)), any(), anyInt(),
- argThat(dexoptOptions -> dexoptOptions.generateAppImage == true));
+ argThat(dexoptOptions -> dexoptOptions.generateAppImage == true), any());
}
private void checkDexoptWithPrivateProfile(
@@ -357,7 +363,7 @@
-> artifacts.permissionSettings.fileFsPermission.isOtherReadable == false),
eq(dexPath), eq(isa), any(), eq("speed-profile"),
deepEq(ProfilePath.tmpRefProfilePath(profile.profilePath)), any(), anyInt(),
- argThat(dexoptOptions -> dexoptOptions.generateAppImage == true));
+ argThat(dexoptOptions -> dexoptOptions.generateAppImage == true), any());
}
private void checkDexoptWithNoProfile(
@@ -366,7 +372,7 @@
argThat(artifacts
-> artifacts.permissionSettings.fileFsPermission.isOtherReadable == true),
eq(dexPath), eq(isa), any(), eq(compilerFilter), isNull(), any(), anyInt(),
- argThat(dexoptOptions -> dexoptOptions.generateAppImage == false));
+ argThat(dexoptOptions -> dexoptOptions.generateAppImage == false), any());
}
private void verifyProfileNotUsed(ProfilePath profile) throws Exception {