Make composd API more async
Allow for notification on success/failure and for early cancellation.
Remove the old fully synchronous API.
Migrate the test to use the API (via modifying the composd_cmd tool).
Bug: 204044765
Test: atest ComposTestCase
Test: manual - make timeout very short, observe cancellation
Change-Id: I1d614ed60bc8baa9d4e58c1ca915e2093aec0808
diff --git a/compos/composd/Android.bp b/compos/composd/Android.bp
index 2a24b7a..8391ed6 100644
--- a/compos/composd/Android.bp
+++ b/compos/composd/Android.bp
@@ -20,6 +20,7 @@
"libnum_traits",
"liblog_rust",
"librustutils",
+ "libshared_child",
],
proc_macros: ["libnum_derive"],
apex_available: [
diff --git a/compos/composd/aidl/android/system/composd/ICompilationTask.aidl b/compos/composd/aidl/android/system/composd/ICompilationTask.aidl
new file mode 100644
index 0000000..ae03fcc
--- /dev/null
+++ b/compos/composd/aidl/android/system/composd/ICompilationTask.aidl
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2021 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 android.system.composd;
+
+/**
+ * Represents a compilation in process.
+ */
+interface ICompilationTask {
+ /**
+ * Attempt to cancel compilation. If successful compilation will end and no further success or
+ * failed callbacks will be received (although any in flight may still be delivered).
+ */
+ void cancel();
+}
diff --git a/compos/composd/aidl/android/system/composd/ICompilationTaskCallback.aidl b/compos/composd/aidl/android/system/composd/ICompilationTaskCallback.aidl
new file mode 100644
index 0000000..a9d41b8
--- /dev/null
+++ b/compos/composd/aidl/android/system/composd/ICompilationTaskCallback.aidl
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2021 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 android.system.composd;
+
+/**
+ * Interface to be implemented by clients of IIsolatedCompilationService to be notified when a
+ * requested compilation task completes.
+ */
+interface ICompilationTaskCallback {
+ /**
+ * Called if a compilation task has ended successfully, generating all the required artifacts.
+ */
+ void onSuccess();
+
+ /**
+ * Called if a compilation task has ended unsuccessfully.
+ */
+ void onFailure();
+}
diff --git a/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl b/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
index 3d0ad31..3d28894 100644
--- a/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
+++ b/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
@@ -15,6 +15,8 @@
*/
package android.system.composd;
+import android.system.composd.ICompilationTask;
+import android.system.composd.ICompilationTaskCallback;
import com.android.compos.CompilationResult;
import com.android.compos.FdAnnotation;
@@ -24,8 +26,11 @@
* This compiles BCP extensions and system server, even if the system artifacts are up to date,
* and writes the results to a test directory to avoid disrupting any real artifacts in
* existence.
+ * Compilation continues in the background, and success/failure is reported via the supplied
+ * callback, unless the returned ICompilationTask is cancelled. The caller should maintain
+ * a reference to the ICompilationTask until compilation completes or is cancelled.
*/
- void runForcedCompileForTest();
+ ICompilationTask startTestCompile(ICompilationTaskCallback callback);
/**
* Run dex2oat in the currently running instance of the CompOS VM. This is a simple proxy
diff --git a/compos/composd/src/compilation_task.rs b/compos/composd/src/compilation_task.rs
new file mode 100644
index 0000000..c4eed52
--- /dev/null
+++ b/compos/composd/src/compilation_task.rs
@@ -0,0 +1,100 @@
+/*
+ * Copyright 2021 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.
+ */
+
+use crate::instance_starter::CompOsInstance;
+use crate::odrefresh::{self, Odrefresh};
+use android_system_composd::aidl::android::system::composd::{
+ ICompilationTask::ICompilationTask, ICompilationTaskCallback::ICompilationTaskCallback,
+};
+use android_system_composd::binder::{Interface, Result as BinderResult, Strong};
+use anyhow::Result;
+use log::{error, warn};
+use std::sync::{Arc, Mutex};
+use std::thread;
+
+#[derive(Clone)]
+pub struct CompilationTask {
+ running_task: Arc<Mutex<Option<RunningTask>>>,
+}
+
+impl Interface for CompilationTask {}
+
+impl ICompilationTask for CompilationTask {
+ fn cancel(&self) -> BinderResult<()> {
+ let task = self.take();
+ if let Some(task) = task {
+ if let Err(e) = task.odrefresh.kill() {
+ warn!("Failed to kill running task: {:?}", e)
+ }
+ }
+ Ok(())
+ }
+}
+
+impl CompilationTask {
+ /// Return the current running task, if any, removing it from this CompilationTask.
+ /// Once removed, meaning the task has ended or been canceled, further calls will always return
+ /// None.
+ fn take(&self) -> Option<RunningTask> {
+ self.running_task.lock().unwrap().take()
+ }
+
+ pub fn start_test_compile(
+ comp_os: Arc<CompOsInstance>,
+ callback: &Strong<dyn ICompilationTaskCallback>,
+ ) -> Result<CompilationTask> {
+ let odrefresh = Odrefresh::spawn_forced_compile("test-artifacts")?;
+ let odrefresh = Arc::new(odrefresh);
+ let task =
+ RunningTask { odrefresh: odrefresh.clone(), comp_os, callback: callback.clone() };
+ let task = CompilationTask { running_task: Arc::new(Mutex::new(Some(task))) };
+
+ task.clone().start_waiting_thread(odrefresh);
+
+ Ok(task)
+ }
+
+ fn start_waiting_thread(self, odrefresh: Arc<Odrefresh>) {
+ thread::spawn(move || {
+ let exit_code = odrefresh.wait_for_exit();
+ let task = self.take();
+ // We don't do the callback if cancel has already happened.
+ if let Some(task) = task {
+ let result = match exit_code {
+ Ok(odrefresh::ExitCode::CompilationSuccess) => task.callback.onSuccess(),
+ Ok(exit_code) => {
+ error!("Unexpected odrefresh result: {:?}", exit_code);
+ task.callback.onFailure()
+ }
+ Err(e) => {
+ error!("Running odrefresh failed: {:?}", e);
+ task.callback.onFailure()
+ }
+ };
+ if let Err(e) = result {
+ warn!("Failed to deliver callback: {:?}", e);
+ }
+ }
+ });
+ }
+}
+
+struct RunningTask {
+ odrefresh: Arc<Odrefresh>,
+ callback: Strong<dyn ICompilationTaskCallback>,
+ #[allow(dead_code)] // Keeps the CompOS VM alive
+ comp_os: Arc<CompOsInstance>,
+}
diff --git a/compos/composd/src/composd_main.rs b/compos/composd/src/composd_main.rs
index 60aeb39..671ed16 100644
--- a/compos/composd/src/composd_main.rs
+++ b/compos/composd/src/composd_main.rs
@@ -18,10 +18,12 @@
//! responsible for managing the lifecycle of the CompOS VM instances, providing key management for
//! them, and orchestrating trusted compilation.
+mod compilation_task;
mod instance_manager;
mod instance_starter;
mod odrefresh;
mod service;
+mod util;
use crate::instance_manager::InstanceManager;
use android_system_composd::binder::{register_lazy_service, ProcessState};
diff --git a/compos/composd/src/instance_starter.rs b/compos/composd/src/instance_starter.rs
index 1a6e592..3959859 100644
--- a/compos/composd/src/instance_starter.rs
+++ b/compos/composd/src/instance_starter.rs
@@ -21,6 +21,7 @@
IVirtualizationService::IVirtualizationService, PartitionType::PartitionType,
};
use anyhow::{bail, Context, Result};
+use binder_common::lazy_service::LazyServiceGuard;
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
use compos_aidl_interface::binder::{ParcelFileDescriptor, Strong};
use compos_common::compos_client::{VmInstance, VmParameters};
@@ -33,9 +34,11 @@
use std::path::{Path, PathBuf};
pub struct CompOsInstance {
+ service: Strong<dyn ICompOsService>,
#[allow(dead_code)] // Keeps VirtualizationService & the VM alive
vm_instance: VmInstance,
- service: Strong<dyn ICompOsService>,
+ #[allow(dead_code)] // Keeps composd process alive
+ lazy_service_guard: LazyServiceGuard,
}
impl CompOsInstance {
@@ -167,7 +170,7 @@
VmInstance::start(virtualization_service, instance_image, &self.vm_parameters)
.context("Starting VM")?;
let service = vm_instance.get_service().context("Connecting to CompOS")?;
- Ok(CompOsInstance { vm_instance, service })
+ Ok(CompOsInstance { vm_instance, service, lazy_service_guard: Default::default() })
}
fn create_instance_image(
diff --git a/compos/composd/src/odrefresh.rs b/compos/composd/src/odrefresh.rs
index 8c3febf..46ea0c0 100644
--- a/compos/composd/src/odrefresh.rs
+++ b/compos/composd/src/odrefresh.rs
@@ -21,6 +21,7 @@
use num_derive::FromPrimitive;
use num_traits::FromPrimitive;
use rustutils::system_properties;
+use shared_child::SharedChild;
use std::process::Command;
// TODO: What if this changes?
@@ -38,30 +39,42 @@
CleanupFailed = EX_MAX + 4,
}
+pub struct Odrefresh {
+ child: SharedChild,
+}
+
fn need_extra_time() -> Result<bool> {
// Special case to add more time in nested VM
let value = system_properties::read("ro.build.product")?;
Ok(value == "vsoc_x86_64" || value == "vsoc_x86")
}
-pub fn run_forced_compile(target_dir: &str) -> Result<ExitCode> {
- // We don`t need to capture stdout/stderr - odrefresh writes to the log
- let mut cmdline = Command::new(ODREFRESH_BIN);
- if need_extra_time()? {
- cmdline.arg("--max-execution-seconds=480").arg("--max-child-process-seconds=150");
+impl Odrefresh {
+ pub fn spawn_forced_compile(target_dir: &str) -> Result<Self> {
+ // We don`t need to capture stdout/stderr - odrefresh writes to the log
+ let mut cmdline = Command::new(ODREFRESH_BIN);
+ if need_extra_time()? {
+ cmdline.arg("--max-execution-seconds=480").arg("--max-child-process-seconds=150");
+ }
+ cmdline
+ .arg(format!("--use-compilation-os={}", VMADDR_CID_ANY as i32))
+ .arg(format!("--dalvik-cache={}", target_dir))
+ .arg("--force-compile");
+ let child = SharedChild::spawn(&mut cmdline).context("Running odrefresh")?;
+ Ok(Odrefresh { child })
}
- cmdline
- .arg(format!("--use-compilation-os={}", VMADDR_CID_ANY as i32))
- .arg(format!("--dalvik-cache={}", target_dir))
- .arg("--force-compile");
- let mut odrefresh = cmdline.spawn().context("Running odrefresh")?;
- // TODO: timeout?
- let status = odrefresh.wait()?;
+ pub fn wait_for_exit(&self) -> Result<ExitCode> {
+ // No timeout here - but clients can kill the process, which will end the wait.
+ let status = self.child.wait()?;
+ if let Some(exit_code) = status.code().and_then(FromPrimitive::from_i32) {
+ Ok(exit_code)
+ } else {
+ bail!("odrefresh exited with {}", status)
+ }
+ }
- if let Some(exit_code) = status.code().and_then(FromPrimitive::from_i32) {
- Ok(exit_code)
- } else {
- bail!("odrefresh exited with {}", status)
+ pub fn kill(&self) -> Result<()> {
+ self.child.kill().context("Killing odrefresh process failed")
}
}
diff --git a/compos/composd/src/service.rs b/compos/composd/src/service.rs
index d3b73a1..351eae9 100644
--- a/compos/composd/src/service.rs
+++ b/compos/composd/src/service.rs
@@ -17,18 +17,20 @@
//! Implementation of IIsolatedCompilationService, called from system server when compilation is
//! desired.
+use crate::compilation_task::CompilationTask;
use crate::instance_manager::InstanceManager;
-use crate::odrefresh;
-use android_system_composd::aidl::android::system::composd::IIsolatedCompilationService::{
- BnIsolatedCompilationService, IIsolatedCompilationService,
+use crate::util::to_binder_result;
+use android_system_composd::aidl::android::system::composd::{
+ ICompilationTask::{BnCompilationTask, ICompilationTask},
+ ICompilationTaskCallback::ICompilationTaskCallback,
+ IIsolatedCompilationService::{BnIsolatedCompilationService, IIsolatedCompilationService},
};
use android_system_composd::binder::{self, BinderFeatures, Interface, Strong};
-use anyhow::{bail, Context, Result};
+use anyhow::{Context, Result};
use binder_common::new_binder_service_specific_error;
use compos_aidl_interface::aidl::com::android::compos::{
CompilationResult::CompilationResult, FdAnnotation::FdAnnotation,
};
-use log::{error, info};
pub struct IsolatedCompilationService {
instance_manager: InstanceManager,
@@ -42,9 +44,12 @@
impl Interface for IsolatedCompilationService {}
impl IIsolatedCompilationService for IsolatedCompilationService {
- fn runForcedCompileForTest(&self) -> binder::Result<()> {
+ fn startTestCompile(
+ &self,
+ callback: &Strong<dyn ICompilationTaskCallback>,
+ ) -> binder::Result<Strong<dyn ICompilationTask>> {
// TODO - check caller is system or shell/root?
- to_binder_result(self.do_run_forced_compile_for_test())
+ to_binder_result(self.do_start_test_compile(callback))
}
fn compile_cmd(
@@ -53,7 +58,7 @@
fd_annotation: &FdAnnotation,
) -> binder::Result<CompilationResult> {
// TODO - check caller is odrefresh
- to_binder_result(self.do_compile(args, fd_annotation))
+ to_binder_result(self.do_compile_cmd(args, fd_annotation))
}
fn compile(&self, _marshaled: &[u8], _fd_annotation: &FdAnnotation) -> binder::Result<i8> {
@@ -61,33 +66,19 @@
}
}
-fn to_binder_result<T>(result: Result<T>) -> binder::Result<T> {
- result.map_err(|e| {
- let message = format!("{:?}", e);
- error!("Returning binder error: {}", &message);
- new_binder_service_specific_error(-1, message)
- })
-}
-
impl IsolatedCompilationService {
- fn do_run_forced_compile_for_test(&self) -> Result<()> {
- info!("runForcedCompileForTest");
-
+ fn do_start_test_compile(
+ &self,
+ callback: &Strong<dyn ICompilationTaskCallback>,
+ ) -> Result<Strong<dyn ICompilationTask>> {
let comp_os = self.instance_manager.start_test_instance().context("Starting CompOS")?;
- let exit_code = odrefresh::run_forced_compile("test-artifacts")?;
+ let task = CompilationTask::start_test_compile(comp_os, callback)?;
- if exit_code != odrefresh::ExitCode::CompilationSuccess {
- bail!("Unexpected odrefresh result: {:?}", exit_code);
- }
-
- // The instance is needed until odrefresh is finished
- drop(comp_os);
-
- Ok(())
+ Ok(BnCompilationTask::new_binder(task, BinderFeatures::default()))
}
- fn do_compile(
+ fn do_compile_cmd(
&self,
args: &[String],
fd_annotation: &FdAnnotation,
diff --git a/compos/composd/src/util.rs b/compos/composd/src/util.rs
new file mode 100644
index 0000000..091fb15
--- /dev/null
+++ b/compos/composd/src/util.rs
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2021 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.
+ */
+
+use android_system_composd::binder::Result as BinderResult;
+use anyhow::Result;
+use binder_common::new_binder_service_specific_error;
+use log::error;
+
+pub fn to_binder_result<T>(result: Result<T>) -> BinderResult<T> {
+ result.map_err(|e| {
+ let message = format!("{:?}", e);
+ error!("Returning binder error: {}", &message);
+ new_binder_service_specific_error(-1, message)
+ })
+}
diff --git a/compos/composd_cmd/composd_cmd.rs b/compos/composd_cmd/composd_cmd.rs
index 04398c0..0b7bd25 100644
--- a/compos/composd_cmd/composd_cmd.rs
+++ b/compos/composd_cmd/composd_cmd.rs
@@ -17,10 +17,16 @@
//! Simple command-line tool to drive composd for testing and debugging.
use android_system_composd::{
- aidl::android::system::composd::IIsolatedCompilationService::IIsolatedCompilationService,
- binder::{wait_for_interface, ProcessState},
+ aidl::android::system::composd::{
+ ICompilationTaskCallback::{BnCompilationTaskCallback, ICompilationTaskCallback},
+ IIsolatedCompilationService::IIsolatedCompilationService,
+ },
+ binder::{wait_for_interface, Interface, ProcessState, Result as BinderResult},
};
-use anyhow::{Context, Result};
+use anyhow::{bail, Context, Result};
+use binder::BinderFeatures;
+use std::sync::{Arc, Condvar, Mutex};
+use std::time::Duration;
fn main() -> Result<()> {
let app = clap::App::new("composd_cmd").arg(
@@ -35,11 +41,8 @@
ProcessState::start_thread_pool();
- let service = wait_for_interface::<dyn IIsolatedCompilationService>("android.system.composd")
- .context("Failed to connect to composd service")?;
-
match command {
- "forced-compile-test" => service.runForcedCompileForTest().context("Compilation failed")?,
+ "forced-compile-test" => run_forced_compile_for_test()?,
_ => panic!("Unexpected command {}", command),
}
@@ -47,3 +50,79 @@
Ok(())
}
+
+struct Callback(Arc<State>);
+
+#[derive(Default)]
+struct State {
+ mutex: Mutex<Option<Outcome>>,
+ completed: Condvar,
+}
+
+#[derive(Copy, Clone)]
+enum Outcome {
+ Succeeded,
+ Failed,
+}
+
+impl Interface for Callback {}
+
+impl ICompilationTaskCallback for Callback {
+ fn onSuccess(&self) -> BinderResult<()> {
+ self.0.set_outcome(Outcome::Succeeded);
+ Ok(())
+ }
+
+ fn onFailure(&self) -> BinderResult<()> {
+ self.0.set_outcome(Outcome::Failed);
+ Ok(())
+ }
+}
+
+impl State {
+ fn set_outcome(&self, outcome: Outcome) {
+ let mut guard = self.mutex.lock().unwrap();
+ *guard = Some(outcome);
+ drop(guard);
+ self.completed.notify_all();
+ }
+
+ fn wait(&self, duration: Duration) -> Result<Outcome> {
+ let (outcome, result) = self
+ .completed
+ .wait_timeout_while(self.mutex.lock().unwrap(), duration, |outcome| outcome.is_none())
+ .unwrap();
+ if result.timed_out() {
+ bail!("Timed out waiting for compilation")
+ }
+ Ok(outcome.unwrap())
+ }
+}
+
+fn run_forced_compile_for_test() -> Result<()> {
+ let service = wait_for_interface::<dyn IIsolatedCompilationService>("android.system.composd")
+ .context("Failed to connect to composd service")?;
+
+ let state = Arc::new(State::default());
+ let callback = Callback(state.clone());
+ let callback = BnCompilationTaskCallback::new_binder(callback, BinderFeatures::default());
+ let task = service.startTestCompile(&callback).context("Compilation failed")?;
+
+ // Make sure composd keeps going even if we don't hold a reference to its service.
+ drop(service);
+
+ // TODO: Handle composd dying without sending callback?
+
+ println!("Waiting");
+
+ match state.wait(Duration::from_secs(480)) {
+ Ok(Outcome::Succeeded) => Ok(()),
+ Ok(Outcome::Failed) => bail!("Compilation failed"),
+ Err(e) => {
+ if let Err(e) = task.cancel() {
+ eprintln!("Failed to cancel compilation: {:?}", e);
+ }
+ Err(e)
+ }
+ }
+}