Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Immutable Event implementation #3198

Merged
merged 18 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions crates/libs/core/src/event.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::*;
use core::cell::UnsafeCell;
use core::ffi::c_void;
use core::marker::PhantomData;
use core::mem::{size_of, transmute_copy};
Expand All @@ -12,9 +13,12 @@ use std::sync::Mutex;
pub struct Event<T: Interface> {
swap: Mutex<()>,
change: Mutex<()>,
delegates: Array<T>,
delegates: UnsafeCell<Array<T>>,
}

unsafe impl<T: Interface> Send for Event<T> {}
unsafe impl<T: Interface> Sync for Event<T> {}
kennykerr marked this conversation as resolved.
Show resolved Hide resolved

impl<T: Interface> Default for Event<T> {
fn default() -> Self {
Self::new()
Expand All @@ -25,47 +29,52 @@ impl<T: Interface> Event<T> {
/// Creates a new, empty `Event<T>`.
pub fn new() -> Self {
Self {
delegates: Array::new(),
delegates: UnsafeCell::new(Array::new()),
swap: Mutex::default(),
change: Mutex::default(),
}
}

/// Registers a delegate with the event object.
pub fn add(&mut self, delegate: &T) -> Result<i64> {
pub fn add(&self, delegate: &T) -> Result<i64> {
let mut _lock_free_drop = Array::new();
Ok({
let _change_lock = self.change.lock().unwrap();
let mut new_delegates = Array::with_capacity(self.delegates.len() + 1)?;
for delegate in self.delegates.as_slice() {
// Safety: there is no mutable alias to self.delegates at this point
kennykerr marked this conversation as resolved.
Show resolved Hide resolved
let current_delegates = unsafe { &*self.delegates.get() };
let mut new_delegates = Array::with_capacity(current_delegates.len() + 1)?;
for delegate in current_delegates.as_slice() {
new_delegates.push(delegate.clone());
}
let delegate = Delegate::new(delegate)?;
let token = delegate.to_token();
new_delegates.push(delegate);

let _swap_lock = self.swap.lock().unwrap();
_lock_free_drop = self.delegates.swap(new_delegates);
// Safety: we have exclusive access to self.delegates at this point
_lock_free_drop = unsafe { &mut *self.delegates.get() }.swap(new_delegates);
token
})
}

/// Revokes a delegate's registration from the event object.
pub fn remove(&mut self, token: i64) -> Result<()> {
pub fn remove(&self, token: i64) -> Result<()> {
let mut _lock_free_drop = Array::new();
{
let _change_lock = self.change.lock().unwrap();
if self.delegates.is_empty() {
// Safety: there is no mutable alias to self.delegates at this point
let current_delegates = unsafe { &*self.delegates.get() };
if current_delegates.is_empty() {
return Ok(());
}
let mut capacity = self.delegates.len() - 1;
let mut capacity = current_delegates.len() - 1;
let mut new_delegates = Array::new();
let mut removed = false;
if capacity == 0 {
removed = self.delegates.as_slice()[0].to_token() == token;
removed = current_delegates.as_slice()[0].to_token() == token;
} else {
new_delegates = Array::with_capacity(capacity)?;
for delegate in self.delegates.as_slice() {
for delegate in current_delegates.as_slice() {
if !removed && delegate.to_token() == token {
removed = true;
continue;
Expand All @@ -79,30 +88,35 @@ impl<T: Interface> Event<T> {
}
if removed {
let _swap_lock = self.swap.lock().unwrap();
_lock_free_drop = self.delegates.swap(new_delegates);
// Safety: we have exclusive access to self.delegates at this point
_lock_free_drop = unsafe { &mut *self.delegates.get() }.swap(new_delegates);
}
}
Ok(())
}

/// Clears the event, removing all delegates.
pub fn clear(&mut self) {
let mut _lock_free_drop = Array::new();
pub fn clear(&self) {
let mut _lock_free_drop = Array::<T>::new();
{
let _change_lock = self.change.lock().unwrap();
if self.delegates.is_empty() {
// Safety: there is no mutable alias to self.delegates at this point
let current_delegates = unsafe { &*self.delegates.get() };
if current_delegates.is_empty() {
return;
}
let _swap_lock = self.swap.lock().unwrap();
_lock_free_drop = self.delegates.swap(Array::new());
// Safety: we have exclusive access to self.delegates at this point
_lock_free_drop = unsafe { &mut *self.delegates.get() }.swap(Array::new());
}
}

/// Invokes all of the event object's registered delegates with the provided callback.
pub fn call<F: FnMut(&T) -> Result<()>>(&mut self, mut callback: F) -> Result<()> {
pub fn call<F: FnMut(&T) -> Result<()>>(&self, mut callback: F) -> Result<()> {
let lock_free_calls = {
let _swap_lock = self.swap.lock().unwrap();
self.delegates.clone()
// Safety: there is no mutable alias to self.delegates at this point
unsafe { &*self.delegates.get() }.clone()
};
for delegate in lock_free_calls.as_slice() {
if let Err(error) = delegate.call(&mut callback) {
Expand All @@ -123,7 +137,6 @@ impl<T: Interface> Event<T> {
struct Array<T: Interface> {
buffer: *mut Buffer<T>,
len: usize,
_phantom: PhantomData<T>,
}

impl<T: Interface> Default for Array<T> {
Expand All @@ -138,7 +151,6 @@ impl<T: Interface> Array<T> {
Self {
buffer: null_mut(),
len: 0,
_phantom: PhantomData,
}
}

Expand All @@ -147,13 +159,12 @@ impl<T: Interface> Array<T> {
Ok(Self {
buffer: Buffer::new(capacity)?,
len: 0,
_phantom: PhantomData,
})
}

/// Swaps the contents of two `Array<T>` objects.
fn swap(&mut self, mut other: Self) -> Self {
unsafe { core::ptr::swap(&mut self.buffer, &mut other.buffer) };
core::mem::swap(&mut self.buffer, &mut other.buffer);
core::mem::swap(&mut self.len, &mut other.len);
other
}
Expand Down Expand Up @@ -203,7 +214,6 @@ impl<T: Interface> Clone for Array<T> {
Self {
buffer: self.buffer,
len: self.len,
_phantom: PhantomData,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/tests/event/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use windows::{core::*, Foundation::*};

#[test]
fn add_remove() -> Result<()> {
let mut event = Event::<EventHandler<i32>>::new();
let event = Event::<EventHandler<i32>>::new();

// Remove a bogus event handler from an empty event source.
event.remove(-123)?;
Expand Down Expand Up @@ -39,7 +39,7 @@ fn add_remove() -> Result<()> {

#[test]
fn multiple() -> Result<()> {
let mut event = Event::<EventHandler<i32>>::new();
let event = Event::<EventHandler<i32>>::new();

let a_check = Arc::new(AtomicI32::new(0));
let a_check_sender = a_check.clone();
Expand Down