gpu_display: gpu_display_win: use builder to create the wndproc thread

Bug: b/291656763
Test: flat run emulator
Change-Id: I5b5597e0ae74556f7d9e3856a265b2ebc49fc77b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4805405
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Colin Downs-Razouk <colindr@google.com>
This commit is contained in:
Kaiyi Li 2023-07-17 15:15:43 -07:00 committed by crosvm LUCI
parent d6e3baf692
commit 4c2892b38e
6 changed files with 79 additions and 30 deletions

View file

@ -1931,19 +1931,3 @@ impl ResourceBridges {
.resize(self.resource_bridges.len(), false);
}
}
/// This function creates the window procedure thread and windows.
///
/// We have seen third-party DLLs hooking into window creation. They may have deep call stack, and
/// they may not be well tested against late window creation, which may lead to stack overflow.
/// Hence, this should be called as early as possible when the VM is booting.
#[cfg(windows)]
#[inline]
pub fn start_wndproc_thread(
#[cfg(feature = "kiwi")] gpu_main_display_tube: Option<Tube>,
) -> anyhow::Result<WindowProcedureThread> {
WindowProcedureThread::start_thread(
#[cfg(feature = "kiwi")]
gpu_main_display_tube,
)
}

View file

@ -176,8 +176,10 @@ pub fn run_gpu_device(opts: Options) -> anyhow::Result<()> {
(&config.params.display_params[0]).into(),
)];
let wndproc_thread =
virtio::gpu::start_wndproc_thread().context("failed to start wndproc_thread")?;
let wndproc_thread_builder = gpu_display::WindowProcedureThread::builder();
let wndproc_thread = wndproc_thread_builder
.start_thread()
.context("failed to start wndproc_thread")?;
let mut gpu_params = config.params.clone();

View file

@ -34,6 +34,7 @@ use vm_control::gpu::DisplayParameters;
use vm_control::ModifyWaitContext;
use window_message_processor::DisplaySendToWndProc;
pub use window_procedure_thread::WindowProcedureThread;
pub use window_procedure_thread::WindowProcedureThreadBuilder;
use crate::DisplayT;
use crate::EventDevice;
@ -327,14 +328,17 @@ mod tests {
.map(|_| {
let (main_ime_tube, _device_ime_tube) =
Tube::pair().expect("failed to create IME tube");
let wndproc_thread_builder = WindowProcedureThread::<TestHandle>::builder();
#[cfg(feature = "kiwi")]
let wndproc_thread_builder = {
let mut wndproc_thread_builder = wndproc_thread_builder;
wndproc_thread_builder
.set_display_tube(None)
.set_ime_tube(Some(_device_ime_tube));
wndproc_thread_builder
};
(
WindowProcedureThread::<TestHandle>::start_thread(
#[cfg(feature = "kiwi")]
None,
#[cfg(feature = "kiwi")]
_device_ime_tube,
)
.unwrap(),
wndproc_thread_builder.start_thread().unwrap(),
main_ime_tube,
)
})

View file

@ -34,6 +34,8 @@ use base::ReadNotifier;
use base::Tube;
use euclid::size2;
use once_cell::sync::OnceCell;
use serde::Deserialize;
use serde::Serialize;
use sync::Mutex;
#[cfg(feature = "kiwi")]
use vm_control::ServiceSendToGpu;
@ -231,9 +233,19 @@ pub struct WindowProcedureThread<T: HandleWindowMessage> {
}
impl<T: HandleWindowMessage> WindowProcedureThread<T> {
pub fn start_thread(
#[cfg(feature = "kiwi")] gpu_main_display_tube: Option<Tube>,
) -> Result<Self> {
pub fn builder() -> WindowProcedureThreadBuilder<T> {
// We don't implement Default for WindowProcedureThreadBuilder so that the builder function
// is the only way to create WindowProcedureThreadBuilder.
WindowProcedureThreadBuilder::<T> {
#[cfg(feature = "kiwi")]
display_tube: None,
#[cfg(feature = "kiwi")]
ime_tube: None,
_marker: Default::default(),
}
}
fn start_thread(#[cfg(feature = "kiwi")] gpu_main_display_tube: Option<Tube>) -> Result<Self> {
let (message_router_handle_sender, message_router_handle_receiver) = channel();
let message_loop_state = Arc::new(AtomicI32::new(MessageLoopState::NotStarted as i32));
let thread_terminated_event = Event::new().unwrap();
@ -624,6 +636,49 @@ impl<T: HandleWindowMessage> Drop for WindowProcedureThread<T> {
// we can implement `Send` for it.
unsafe impl<T: HandleWindowMessage> Send for WindowProcedureThread<T> {}
#[derive(Deserialize, Serialize)]
pub struct WindowProcedureThreadBuilder<T: HandleWindowMessage> {
#[cfg(feature = "kiwi")]
display_tube: Option<Tube>,
#[cfg(feature = "kiwi")]
ime_tube: Option<Tube>,
// We use fn(T) -> T here so that this struct is still Send + Sync regardless of whether T is
// Send + Sync. See details of this pattern at
// https://doc.rust-lang.org/nomicon/phantom-data.html#table-of-phantomdata-patterns.
_marker: PhantomData<fn(T) -> T>,
}
impl<T: HandleWindowMessage> WindowProcedureThreadBuilder<T> {
#[cfg(feature = "kiwi")]
pub fn set_display_tube(&mut self, display_tube: Option<Tube>) -> &mut Self {
self.display_tube = display_tube;
self
}
#[cfg(feature = "kiwi")]
pub fn set_ime_tube(&mut self, ime_tube: Option<Tube>) -> &mut Self {
self.ime_tube = ime_tube;
self
}
/// This function creates the window procedure thread and windows.
///
/// We have seen third-party DLLs hooking into window creation. They may have deep call stack,
/// and they may not be well tested against late window creation, which may lead to stack
/// overflow. Hence, this should be called as early as possible when the VM is booting.
pub fn start_thread(self) -> Result<WindowProcedureThread<T>> {
cfg_if::cfg_if! {
if #[cfg(feature = "kiwi")] {
let Self { ime_tube, display_tube, .. } = self;
let ime_tube = ime_tube.ok_or_else(|| anyhow!("The ime tube is not set."))?;
WindowProcedureThread::<T>::start_thread(display_tube, ime_tube)
} else {
WindowProcedureThread::<T>::start_thread()
}
}
}
}
#[cfg(test)]
mod tests {
use super::*;

View file

@ -41,6 +41,9 @@ use sys::SysDisplayT;
pub use sys::SysGpuDisplayExt;
#[cfg(windows)]
pub type WindowProcedureThread = gpu_display_win::WindowProcedureThread<gpu_display_win::Surface>;
#[cfg(windows)]
pub type WindowProcedureThreadBuilder =
gpu_display_win::WindowProcedureThreadBuilder<gpu_display_win::Surface>;
/// An error generated by `GpuDisplay`.
#[sorted]

View file

@ -213,8 +213,9 @@ pub(super) fn create_gpu(
features: u64,
_product_args: GpuBackendConfigProduct,
) -> Result<Gpu> {
let wndproc_thread =
virtio::gpu::start_wndproc_thread().expect("Failed to start wndproc_thread!");
let wndproc_thread = gpu_display::WindowProcedureThread::builder()
.start_thread()
.expect("Failed to start wndproc_thread!");
Ok(Gpu::new(
vm_evt_wrtube