Skip to content

Commit df7ebac

Browse files
George-MiaoGeorge-Miao
authored
refactor(driver): require Default for Control (#859)
1 parent fb25ee9 commit df7ebac

20 files changed

Lines changed: 536 additions & 511 deletions

File tree

‎compio-driver/Cargo.toml‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ flume = { workspace = true, default-features = false }
2626
futures-util = { workspace = true }
2727
socket2 = { workspace = true, features = ["all"] }
2828
thin-cell = { workspace = true }
29-
smallvec = { workspace = true, optional = true, features = ["union"] }
3029
synchrony = { workspace = true, features = ["waker_slot"] }
3130
bitflags = { version = "2.11.0" }
3231
paste = { workspace = true }
@@ -57,9 +56,9 @@ polling = { version = "3.3.0", optional = true }
5756
# Other platform dependencies
5857
[target.'cfg(all(not(target_os = "linux"), unix))'.dependencies]
5958
polling = "3.3.0"
60-
smallvec = { workspace = true }
6159

6260
[target.'cfg(unix)'.dependencies]
61+
smallvec = { workspace = true, features = ["union"] }
6362
crossbeam-queue = { workspace = true }
6463
libc = { workspace = true }
6564
nix = { workspace = true, features = ["fs"] }
@@ -77,7 +76,7 @@ io-uring = [
7776
"dep:io-uring",
7877
"dep:once_cell",
7978
]
80-
polling = ["dep:polling", "dep:smallvec"]
79+
polling = ["dep:polling"]
8180

8281
sync = []
8382

‎compio-driver/src/control.rs‎

Lines changed: 29 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
#![allow(dead_code)]
22

3-
use std::{
4-
mem::{ManuallyDrop, MaybeUninit},
5-
ptr,
6-
};
7-
83
use compio_buf::IntoInner;
94

105
use crate::{DriverType, OpCode};
@@ -52,36 +47,33 @@ impl<T: OpCode> ControlInner<T> {
5247
///
5348
/// [`ErasedKey`]: crate::key::ErasedKey
5449
pub(crate) struct Carrier<T: OpCode + ?Sized> {
55-
control: MaybeUninit<ControlInner<T>>,
50+
control: ControlInner<T>,
51+
driver_ty: DriverType,
5652
op: T,
5753
}
5854

5955
impl<T: OpCode> IntoInner for Carrier<T> {
6056
type Inner = T;
6157

6258
fn into_inner(self) -> Self::Inner {
63-
let mut this = ManuallyDrop::new(self);
64-
unsafe {
65-
// SAFETY: `new_uninit` ensures that the type is initialized
66-
MaybeUninit::assume_init_drop(&mut this.control);
67-
// SAFETY: `self` is warpped in ManuallyDrop and is not used after here
68-
ptr::read(&this.op)
69-
}
59+
self.op
7060
}
7161
}
7262

7363
impl<T: OpCode> Carrier<T> {
74-
/// Create a new Carrier with given OpCode.
75-
///
76-
/// # Safety
77-
///
78-
/// Returned [`Carrier`] must be [`initialized`] before converting to `dyn
79-
/// Carry` or calling any of the `as_` getters.
80-
///
81-
/// [`initialized`]: Self::init
82-
pub unsafe fn new_uninit(op: T) -> Self {
64+
/// Create a new Carrier with given OpCode and driver type.
65+
pub fn new(op: T, driver_ty: DriverType) -> Self {
66+
#[cfg(fusion)]
67+
let control = match driver_ty {
68+
DriverType::IoUring => ControlInner::IoUring(Default::default()),
69+
DriverType::Poll => ControlInner::Poll(Default::default()),
70+
_ => unreachable!("Cannot be IOCP"),
71+
};
72+
#[cfg(not(fusion))]
73+
let control = T::Control::default();
8374
Self {
84-
control: MaybeUninit::uninit(),
75+
control,
76+
driver_ty,
8577
op,
8678
}
8779
}
@@ -92,77 +84,45 @@ impl<T: OpCode> Carrier<T> {
9284
///
9385
/// `self` must have stable address until control is no longer used, include
9486
/// all function calls via `dyn Carry` and `as_` getters.
95-
pub unsafe fn init(&mut self, driver_ty: DriverType) {
87+
pub unsafe fn init(&mut self) {
9688
#[cfg(fusion)]
97-
{
98-
let control = match driver_ty {
99-
DriverType::Poll => ControlInner::Poll(unsafe { PollOpCode::init(&mut self.op) }),
100-
DriverType::IoUring => {
101-
ControlInner::IoUring(unsafe { IourOpCode::init(&mut self.op) })
102-
}
103-
DriverType::IOCP => unreachable!("Cannot be windows"),
89+
unsafe {
90+
match self.driver_ty {
91+
DriverType::Poll => PollOpCode::init(&mut self.op, self.control.poll()),
92+
DriverType::IoUring => IourOpCode::init(&mut self.op, self.control.iour()),
93+
_ => unreachable!("Cannot be IOCP"),
10494
};
105-
106-
self.control.write(control);
10795
}
10896

10997
#[cfg(not(fusion))]
110-
{
111-
_ = driver_ty;
112-
113-
let control = unsafe { OpCode::init(&mut self.op) };
114-
self.control.write(control);
98+
unsafe {
99+
OpCode::init(&mut self.op, &mut self.control)
115100
}
116101
}
117102

118103
#[cfg(io_uring)]
119104
pub fn as_iour(&mut self) -> (&mut T, &mut <T as IourOpCode>::Control) {
120-
// SAFETY: `new_uninit` ensures that the type is initialized
121-
let control = unsafe { self.control.assume_init_mut() };
122105
#[cfg(fusion)]
123-
{
124-
(&mut self.op, control.iour())
125-
}
106+
return (&mut self.op, self.control.iour());
126107
#[cfg(not(fusion))]
127-
{
128-
(&mut self.op, control)
129-
}
108+
(&mut self.op, &mut self.control)
130109
}
131110

132111
#[cfg(polling)]
133112
pub fn as_poll(&mut self) -> (&mut T, &mut <T as PollOpCode>::Control) {
134-
// SAFETY: `new_uninit` ensures that the type is initialized
135-
let control = unsafe { self.control.assume_init_mut() };
136113
#[cfg(fusion)]
137-
{
138-
(&mut self.op, control.poll())
139-
}
114+
return (&mut self.op, self.control.poll());
140115
#[cfg(not(fusion))]
141-
{
142-
(&mut self.op, control)
143-
}
116+
(&mut self.op, &mut self.control)
144117
}
145118

146119
#[cfg(windows)]
147120
pub fn as_iocp(&self) -> (&T, &<T as OpCode>::Control) {
148-
// SAFETY: `new_uninit` ensures that the type is initialized
149-
let control = unsafe { self.control.assume_init_ref() };
150-
151-
(&self.op, control)
121+
(&self.op, &self.control)
152122
}
153123

154124
#[cfg(windows)]
155125
pub fn as_iocp_mut(&mut self) -> (&mut T, &mut <T as OpCode>::Control) {
156-
// SAFETY: `new_uninit` ensures that the type is initialized
157-
let control = unsafe { self.control.assume_init_mut() };
158-
159-
(&mut self.op, control)
160-
}
161-
}
162-
163-
impl<T: OpCode + ?Sized> Drop for Carrier<T> {
164-
fn drop(&mut self) {
165-
// SAFETY: `new_uninit` ensures that the type is initialized
166-
unsafe { MaybeUninit::assume_init_drop(&mut self.control) };
126+
(&mut self.op, &mut self.control)
167127
}
168128
}

‎compio-driver/src/key.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,15 +211,14 @@ impl ErasedKey {
211211
extra,
212212
cancelled: false,
213213
result: PushEntry::Pending(None),
214-
// SAFETY: carrier is initialized below
215-
carrier: unsafe { Carrier::new_uninit(op) },
214+
carrier: Carrier::new(op, driver_ty),
216215
};
217216
let mut inner = ThinCell::new(raw_op);
218217
// SAFETY:
219218
// - ThinCell is just created, there will be no shared owner or borrower
220219
// - Carrier is being pinned by ThinCell, it will have a stable address until
221220
// move out
222-
unsafe { inner.borrow_unchecked().carrier.init(driver_ty) };
221+
unsafe { inner.borrow_unchecked().carrier.init() };
223222
Self {
224223
inner: unsafe { inner.unsize(|p| p as *const Inner<RawOp<dyn Carry>>) },
225224
}

‎compio-driver/src/sys/fusion/op.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ macro_rules! op {
8181
unsafe impl<$($ty: $trait),*> poll::OpCode for $name<$($ty),*> {
8282
type Control = <poll::$name<$($ty),*> as poll::OpCode>::Control;
8383

84-
unsafe fn init(&mut self) -> Self::Control {
85-
unsafe { poll::OpCode::init(self.inner.poll()) }
84+
unsafe fn init(&mut self, ctrl: &mut Self::Control) {
85+
unsafe { poll::OpCode::init(self.inner.poll(), ctrl) }
8686
}
8787

8888
fn pre_submit(&mut self, control: &mut Self::Control) -> std::io::Result<crate::Decision> {
@@ -103,8 +103,8 @@ macro_rules! op {
103103
unsafe impl<$($ty: $trait),*> iour::OpCode for $name<$($ty),*> {
104104
type Control = <iour::$name<$($ty),*> as iour::OpCode>::Control;
105105

106-
unsafe fn init(&mut self) -> Self::Control {
107-
unsafe { self.inner.iour().init() }
106+
unsafe fn init(&mut self, ctrl: &mut Self::Control) {
107+
unsafe { self.inner.iour().init(ctrl) }
108108
}
109109

110110
fn create_entry(&mut self, control: &mut Self::Control) -> OpEntry {
@@ -214,8 +214,8 @@ macro_rules! mop {
214214
unsafe impl<$($ty: $trait),*> poll::OpCode for $name<$($ty),*> {
215215
type Control = <crate::op::managed::$name<$($ty),*> as poll::OpCode>::Control;
216216

217-
unsafe fn init(&mut self) -> Self::Control {
218-
unsafe { self.inner.poll().init() }
217+
unsafe fn init(&mut self, ctrl: &mut Self::Control) {
218+
unsafe { self.inner.poll().init(ctrl) }
219219
}
220220

221221
fn pre_submit(&mut self, control: &mut Self::Control) -> std::io::Result<crate::Decision> {
@@ -236,8 +236,8 @@ macro_rules! mop {
236236
unsafe impl<$($ty: $trait),*> iour::OpCode for $name<$($ty),*> {
237237
type Control = <iour::$name<$($ty),*> as iour::OpCode>::Control;
238238

239-
unsafe fn init(&mut self) -> Self::Control {
240-
unsafe { self.inner.iour().init() }
239+
unsafe fn init(&mut self, ctrl: &mut Self::Control) {
240+
unsafe { self.inner.iour().init(ctrl) }
241241
}
242242

243243
fn create_entry(&mut self, control: &mut Self::Control) -> OpEntry {

‎compio-driver/src/sys/iocp/mod.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -294,15 +294,15 @@ pub enum OpType {
294294
pub unsafe trait OpCode {
295295
/// Type that contains self-references and other needed info during the
296296
/// operation
297-
type Control;
297+
type Control: Default;
298298

299-
/// Constructs a `Control`
299+
/// Initialize the control
300300
///
301301
/// # Safety
302302
///
303-
/// Caller must guarantee that during the lifetime of the returned
304-
/// `Control`, `Self` must be unmoved and valid.
305-
unsafe fn init(&mut self) -> Self::Control;
303+
/// Caller must guarantee that during the lifetime of `ctrl`, `Self` is
304+
/// unmoved and valid.
305+
unsafe fn init(&mut self, ctrl: &mut Self::Control);
306306

307307
/// Determines that the operation is really overlapped defined by Windows
308308
/// API. If not, the driver will try to operate it in another thread.

0 commit comments

Comments
 (0)