From bfca527582d72f628f5703901e0e0123b1630ed2 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 04:10:03 -0400 Subject: [PATCH 01/27] Add RwLock impl --- src/intrusive_double_linked_list.rs | 61 ++ src/sync/mod.rs | 2 + src/sync/rwlock.rs | 1324 +++++++++++++++++++++++++++ 3 files changed, 1387 insertions(+) create mode 100644 src/sync/rwlock.rs diff --git a/src/intrusive_double_linked_list.rs b/src/intrusive_double_linked_list.rs index f87f3e1..beaf875 100644 --- a/src/intrusive_double_linked_list.rs +++ b/src/intrusive_double_linked_list.rs @@ -321,6 +321,67 @@ impl LinkedList { } } } + + /// Iterate the list in reverse order by calling a callback on each list node + /// and determining what to do based on the control flow. + pub fn reverse_apply_while(&mut self, mut func: F) + where + F: FnMut(&mut ListNode) -> ControlFlow, + { + let mut current = self.tail; + + while let Some(mut node) = current { + unsafe { + let flow = func(node.as_mut()); + match flow { + ControlFlow::Continue => { + current = node.as_mut().prev; + } + ControlFlow::Stop => return, + ControlFlow::RemoveAndStop + | ControlFlow::RemoveAndContinue => { + let node = node.as_mut(); + match node.prev { + Some(mut prev) => { + prev.as_mut().next = node.next; + } + None => { + self.head = node.next; + } + } + match node.next { + Some(mut next) => { + next.as_mut().prev = node.prev; + } + None => { + self.tail = node.prev; + } + } + if let ControlFlow::RemoveAndStop = flow { + return; + } else { + current = node.prev; + } + } + } + } + } + } +} + +/// The outcome of a callback. +pub enum ControlFlow { + /// Continue the iteration. + Continue, + + /// Stop the iteration. + Stop, + + /// Remove the current entry and stop the iteration. + RemoveAndStop, + + /// Remove the current entry and continue the iteration. + RemoveAndContinue, } #[cfg(all(test, feature = "alloc"))] // Tests make use of Vec at the moment diff --git a/src/sync/mod.rs b/src/sync/mod.rs index b4a11bf..54b7f28 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -41,3 +41,5 @@ pub use self::semaphore::{ Semaphore, SemaphoreAcquireFuture, SemaphoreReleaser, SharedSemaphore, SharedSemaphoreAcquireFuture, SharedSemaphoreReleaser, }; + +mod rwlock; diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs new file mode 100644 index 0000000..dd2d3a4 --- /dev/null +++ b/src/sync/rwlock.rs @@ -0,0 +1,1324 @@ +//! An asynchronously awaitable mutex for synchronization between concurrently +//! executing futures. + +use crate::{ + intrusive_double_linked_list::{ControlFlow, LinkedList, ListNode}, + utils::update_waker_ref, + NoopLock, +}; +use core::{ + cell::UnsafeCell, + ops::{Deref, DerefMut}, + pin::Pin, +}; +use futures_core::{ + future::{FusedFuture, Future}, + task::{Context, Poll, Waker}, +}; +use lock_api::{Mutex as LockApiMutex, RawMutex}; + +/// Tracks how the future had interacted with the mutex +#[derive(PartialEq)] +enum PollState { + /// The task has never interacted with the mutex. + New, + + /// The task was added to the wait queue at the mutex. + Waiting, + + /// The task had previously waited on the mutex, but was notified + /// that the mutex was released in the meantime. + Notified, + + /// The task had been polled to completion. + Done, +} + +/// The kind of entry. +#[derive(Debug, PartialEq)] +enum EntryKind { + /// An exclusive write access. + Write, + + /// A shared read access. + Read, + + /// A upgradable shared read access. + UpgradeRead, +} + +/// Tracks the MutexLockFuture waiting state. +/// Access to this struct is synchronized through the mutex in the Event. +struct Entry { + /// The task handle of the waiting task + task: Option, + + /// Current polling state + state: PollState, + + /// The kind of entry. + kind: EntryKind, +} + +impl Entry { + /// Creates a new Entry + fn new(kind: EntryKind) -> Entry { + Entry { + task: None, + state: PollState::New, + kind, + } + } + + /// Update the state & wakeup if possible. + fn notify(&mut self) { + self.state = PollState::Notified; + + if let Some(waker) = self.task.take() { + waker.wake(); + } + } +} + +/// Internal state of the `Mutex` +struct MutexState { + is_fair: bool, + nb_reads: usize, + has_upgrade_read: bool, + has_write: bool, + waiters: LinkedList, + nb_waiting_writes: usize, + nb_waiting_reads: usize, + nb_waiting_upgrade_reads: usize, +} + +impl MutexState { + fn new(is_fair: bool) -> Self { + MutexState { + is_fair, + nb_reads: 0, + has_upgrade_read: false, + has_write: false, + waiters: LinkedList::new(), + nb_waiting_writes: 0, + nb_waiting_reads: 0, + nb_waiting_upgrade_reads: 0, + } + } + + /// Reduce the number of reads on the lock by one, waking + /// up a write if needed. + /// + /// If the Mutex is not fair, removes the woken up node from + /// the wait queue + fn unlock_read(&mut self) { + self.nb_reads -= 1; + if self.nb_reads == 0 { + // Wakeup the next write in line. + if self.is_fair { + self.waiters.reverse_apply_while(|entry| { + if let EntryKind::Write = entry.kind { + entry.notify(); + ControlFlow::Stop + } else { + ControlFlow::Continue + } + }); + } else { + self.waiters.reverse_apply_while(|entry| { + if let EntryKind::Write = entry.kind { + entry.notify(); + ControlFlow::RemoveAndStop + } else { + ControlFlow::Continue + } + }); + }; + } + } + + /// Release the exclusive writer lock, waking up entries + /// if needed. + /// + /// If the Mutex is not fair, removes the woken up node from + /// the wait queue + fn unlock_write(&mut self) { + self.has_write = false; + debug_assert_eq!(self.nb_reads, 0); + + self.wakeup_any_waiters() + } + + /// Release the upgradable reader lock, waking up entries + /// if needed. + /// + /// If the Mutex is not fair, removes the woken up node from + /// the wait queue + fn unlock_upgrade_read(&mut self) { + self.has_upgrade_read = false; + self.nb_reads -= 1; + + if self.nb_reads == 0 { + self.wakeup_any_waiters(); + } + } + + /// Release the upgradable reader lock to upgrade + /// it into a writer lock, waking up entries if needed. + /// + /// If the Mutex is not fair does not wakeup anyone. + fn unlock_upgrade_write(&mut self) { + self.has_upgrade_read = false; + self.nb_reads -= 1; + + if self.nb_reads == 0 && self.is_fair { + self.wakeup_any_waiters(); + } + } + + fn wakeup_any_waiters(&mut self) { + let mut nb_reads = 0; + if self.is_fair { + self.waiters.reverse_apply_while(|entry| { + match entry.kind { + EntryKind::Read | EntryKind::UpgradeRead => { + entry.notify(); + nb_reads += 1; + ControlFlow::Continue + } + EntryKind::Write => { + if nb_reads == 0 { + entry.notify(); + ControlFlow::Stop + } else { + ControlFlow::Continue + } + } + } + }); + } else { + self.waiters.reverse_apply_while(|entry| { + match entry.kind { + EntryKind::Read | EntryKind::UpgradeRead => { + entry.notify(); + nb_reads += 1; + ControlFlow::RemoveAndContinue + } + EntryKind::Write => { + if nb_reads == 0 { + entry.notify(); + ControlFlow::RemoveAndStop + } else { + ControlFlow::Continue + } + } + } + }); + }; + } + + /// Attempt To gain shared read access. + /// + /// Returns true if the access is obtained. + fn try_lock_read_sync(&mut self) -> bool { + // The lock can only be obtained synchronously if + // - has no write + // - the Semaphore is either not fair, or there are no waiting writes. + if !self.has_write && (!self.is_fair || self.nb_waiting_writes == 0) { + self.nb_reads += 1; + true + } else { + false + } + } + + /// Attempt To gain upgradable shared read access. + /// + /// Returns true if the access is obtained. + fn try_lock_upgrade_read_sync(&mut self) -> bool { + // The lock can only be obtained synchronously if + // - has no write + // - the Semaphore is either not fair, or there are no waiting writes + // or upgradable reads + if !self.has_write + && !self.has_upgrade_read + && (!self.is_fair + || (self.nb_waiting_writes == 0 + && self.nb_waiting_upgrade_reads == 0)) + { + self.nb_reads += 1; + self.has_upgrade_read = true; + true + } else { + false + } + } + + /// Attempt to gain exclusive write access. + /// + /// Returns true if the access is obtained. + fn try_lock_write_sync(&mut self) -> bool { + // The lock can only be obtained synchronously if + // - has no write + // - has no read + // - the Semaphore is either not fair, or there are no waiting writes. + if !self.has_write + && self.nb_reads == 0 + && (!self.is_fair + || (self.nb_waiting_writes == 0 + && self.nb_waiting_upgrade_reads == 0)) + { + self.has_write = true; + true + } else { + false + } + } + + /// Attempt to gain exclusive write access. + /// + /// Returns true if the access is obtained. + fn try_upgrade_write_sync(&mut self) -> bool { + // The lock can only be obtained synchronously if + // - has no write + // - has 1 read (the caller) + // - the Semaphore is either not fair, or there are no waiting writes. + debug_assert!(self.has_upgrade_read); + if !self.has_write + && self.nb_reads == 1 + && (!self.is_fair + || (self.nb_waiting_writes == 0 + && self.nb_waiting_upgrade_reads == 0)) + { + self.has_write = true; + self.nb_reads -= 1; + self.has_upgrade_read = false; + true + } else { + false + } + } + + /// Add a read to the wait queue. + /// + /// Safety: This function is only safe as long as `node` is guaranteed to + /// get removed from the list before it gets moved or dropped. + /// In addition to this `node` may not be added to another other list before + /// it is removed from the current one. + unsafe fn add_read(&mut self, wait_node: &mut ListNode) { + debug_assert_eq!(wait_node.kind, EntryKind::Read); + self.waiters.add_front(wait_node); + self.nb_waiting_reads += 1; + } + + /// Add a write to the wait queue. + /// + /// Safety: This function is only safe as long as `node` is guaranteed to + /// get removed from the list before it gets moved or dropped. + /// In addition to this `node` may not be added to another other list before it is removed from the current one. + unsafe fn add_write(&mut self, wait_node: &mut ListNode) { + debug_assert_eq!(wait_node.kind, EntryKind::Write); + self.waiters.add_front(wait_node); + self.nb_waiting_writes += 1; + } + + /// Add a write to the wait queue. + /// + /// Safety: This function is only safe as long as `node` is guaranteed to + /// get removed from the list before it gets moved or dropped. + /// In addition to this `node` may not be added to another other list before + /// it is removed from the current one. + unsafe fn add_upgrade_read(&mut self, wait_node: &mut ListNode) { + debug_assert_eq!(wait_node.kind, EntryKind::Write); + self.waiters.add_front(wait_node); + self.nb_waiting_upgrade_reads += 1; + } + + /// Tries to acquire the shared read access from a Entry. + /// + /// If it isn't available, the Entry gets added to the wait + /// queue at the Mutex, and will be signalled once ready. + /// This function is only safe as long as the `wait_node`s address is guaranteed + /// to be stable until it gets removed from the queue. + unsafe fn try_lock_read( + &mut self, + wait_node: &mut ListNode, + cx: &mut Context<'_>, + ) -> Poll<()> { + match wait_node.state { + PollState::New => { + // The fast path - the Mutex isn't locked by anyone else. + // If the mutex is fair, noone must be in the wait list before us. + if self.try_lock_read_sync() { + wait_node.state = PollState::Done; + Poll::Ready(()) + } else { + // Add the task to the wait queue + wait_node.task = Some(cx.waker().clone()); + wait_node.state = PollState::Waiting; + Poll::Pending + } + } + PollState::Waiting => { + // The RwLockReadFuture is already in the queue. + if self.is_fair { + // The task needs to wait until it gets notified in order to + // maintain the ordering. However the caller might have + // passed a different `Waker`. In this case we need to update it. + update_waker_ref(&mut wait_node.task, cx); + Poll::Pending + } else { + // For throughput improvement purposes, grab the lock immediately + // if it's available. + if !self.has_write { + self.nb_reads += 1; + wait_node.state = PollState::Done; + // Since this waiter has been registered before, it must + // get removed from the waiter list. + // Safety: Due to the state, we know that the node must be part + // of the waiter list + self.force_remove_read(wait_node); + Poll::Ready(()) + } else { + // The caller might have passed a different `Waker`. + // In this case we need to update it. + update_waker_ref(&mut wait_node.task, cx); + Poll::Pending + } + } + } + PollState::Notified => { + // We had been woken by the mutex, since the mutex is available again. + // The mutex thereby removed us from the waiters list. + // Just try to lock again. If the mutex isn't available, + // we need to add it to the wait queue again. + if !self.has_write { + if self.is_fair { + // In a fair Mutex, the Entry is kept in the + // linked list and must be removed here + // Safety: Due to the state, we know that the node must be part + // of the waiter list + self.force_remove_read(wait_node); + } + self.nb_reads += 1; + wait_node.state = PollState::Done; + Poll::Ready(()) + } else { + // Fair mutexes should always be able to acquire the lock + // after they had been notified + debug_assert!(!self.is_fair); + // Add to queue + wait_node.task = Some(cx.waker().clone()); + wait_node.state = PollState::Waiting; + self.add_read(wait_node); + Poll::Pending + } + } + PollState::Done => { + // The future had been polled to completion before + panic!("polled Mutex after completion"); + } + } + } + + /// Tries to acquire an exclusive write access from a Entry. + /// + /// If it isn't available, the Entry gets added to the wait + /// queue at the Mutex, and will be signalled once ready. + /// This function is only safe as long as the `wait_node`s address is guaranteed + /// to be stable until it gets removed from the queue. + unsafe fn try_lock_write( + &mut self, + wait_node: &mut ListNode, + cx: &mut Context<'_>, + ) -> Poll<()> { + match wait_node.state { + PollState::New => { + // The fast path - the Mutex isn't locked by anyone else. + // If the mutex is fair, noone must be in the wait list before us. + if self.try_lock_write_sync() { + wait_node.state = PollState::Done; + Poll::Ready(()) + } else { + // Add the task to the wait queue + wait_node.task = Some(cx.waker().clone()); + wait_node.state = PollState::Waiting; + self.add_write(wait_node); + Poll::Pending + } + } + PollState::Waiting => { + // The RwLockReadFuture is already in the queue. + if self.is_fair { + // The task needs to wait until it gets notified in order to + // maintain the ordering. However the caller might have + // passed a different `Waker`. In this case we need to update it. + update_waker_ref(&mut wait_node.task, cx); + Poll::Pending + } else { + // For throughput improvement purposes, grab the lock immediately + // if it's available. + if !self.has_write && self.nb_reads == 0 { + self.has_write = true; + wait_node.state = PollState::Done; + // Since this waiter has been registered before, it must + // get removed from the waiter list. + // Safety: Due to the state, we know that the node must be part + // of the waiter list + self.force_remove_write(wait_node); + Poll::Ready(()) + } else { + // The caller might have passed a different `Waker`. + // In this case we need to update it. + update_waker_ref(&mut wait_node.task, cx); + Poll::Pending + } + } + } + PollState::Notified => { + // We had been woken by the mutex, since the mutex is available again. + // The mutex thereby removed us from the waiters list. + // Just try to lock again. If the mutex isn't available, + // we need to add it to the wait queue again. + if !self.has_write && self.nb_reads == 0 { + if self.is_fair { + // In a fair Mutex, the Entry is kept in the + // linked list and must be removed here + // Safety: Due to the state, we know that the node must be part + // of the waiter list + self.force_remove_write(wait_node); + } + self.has_write = true; + wait_node.state = PollState::Done; + Poll::Ready(()) + } else { + // Fair mutexes should always be able to acquire the lock + // after they had been notified + debug_assert!(!self.is_fair); + // Add to queue + wait_node.task = Some(cx.waker().clone()); + wait_node.state = PollState::Waiting; + self.add_write(wait_node); + Poll::Pending + } + } + PollState::Done => { + // The future had been polled to completion before + panic!("polled Mutex after completion"); + } + } + } + + /// Tries to acquire an upgradable shared read access from a Entry. + /// + /// If it isn't available, the Entry gets added to the wait + /// queue at the Mutex, and will be signalled once ready. + /// This function is only safe as long as the `wait_node`s address is guaranteed + /// to be stable until it gets removed from the queue. + unsafe fn try_lock_upgrade_read( + &mut self, + wait_node: &mut ListNode, + cx: &mut Context<'_>, + ) -> Poll<()> { + match wait_node.state { + PollState::New => { + // The fast path - the Mutex isn't locked by anyone else. + // If the mutex is fair, noone must be in the wait list before us. + if self.try_lock_upgrade_read_sync() { + wait_node.state = PollState::Done; + Poll::Ready(()) + } else { + // Add the task to the wait queue + wait_node.task = Some(cx.waker().clone()); + wait_node.state = PollState::Waiting; + self.add_upgrade_read(wait_node); + Poll::Pending + } + } + PollState::Waiting => { + // The RwLockReadFuture is already in the queue. + if self.is_fair { + // The task needs to wait until it gets notified in order to + // maintain the ordering. However the caller might have + // passed a different `Waker`. In this case we need to update it. + update_waker_ref(&mut wait_node.task, cx); + Poll::Pending + } else { + // For throughput improvement purposes, grab the lock immediately + // if it's available. + if !self.has_write && !self.has_upgrade_read { + self.has_upgrade_read = true; + self.nb_reads += 1; + wait_node.state = PollState::Done; + // Since this waiter has been registered before, it must + // get removed from the waiter list. + // Safety: Due to the state, we know that the node must be part + // of the waiter list + self.force_remove_upgrade_read(wait_node); + Poll::Ready(()) + } else { + // The caller might have passed a different `Waker`. + // In this case we need to update it. + update_waker_ref(&mut wait_node.task, cx); + Poll::Pending + } + } + } + PollState::Notified => { + // We had been woken by the mutex, since the mutex is available again. + // The mutex thereby removed us from the waiters list. + // Just try to lock again. If the mutex isn't available, + // we need to add it to the wait queue again. + if !self.has_write && !self.has_upgrade_read { + if self.is_fair { + // In a fair Mutex, the Entry is kept in the + // linked list and must be removed here + // Safety: Due to the state, we know that the node must be part + // of the waiter list + self.force_remove_upgrade_read(wait_node); + } + self.has_upgrade_read = true; + self.nb_reads += 1; + wait_node.state = PollState::Done; + Poll::Ready(()) + } else { + // Fair mutexes should always be able to acquire the lock + // after they had been notified + debug_assert!(!self.is_fair); + // Add to queue + wait_node.task = Some(cx.waker().clone()); + wait_node.state = PollState::Waiting; + self.add_upgrade_read(wait_node); + Poll::Pending + } + } + PollState::Done => { + // The future had been polled to completion before + panic!("polled Mutex after completion"); + } + } + } + + /// Tries to remove a read waiter from the wait queue, and panics if the + /// waiter is no longer valid. + unsafe fn force_remove_read(&mut self, wait_node: &mut ListNode) { + if !self.waiters.remove(wait_node) { + // Panic if the address isn't found. This can only happen if the contract was + // violated, e.g. the Entry got moved after the initial poll. + panic!("Future could not be removed from wait queue"); + } + self.nb_waiting_reads -= 1; + } + + /// Tries to remove a write waiter from the wait queue, and panics if the + /// waiter is no longer valid. + unsafe fn force_remove_write(&mut self, wait_node: &mut ListNode) { + if !self.waiters.remove(wait_node) { + // Panic if the address isn't found. This can only happen if the contract was + // violated, e.g. the Entry got moved after the initial poll. + panic!("Future could not be removed from wait queue"); + } + self.nb_waiting_writes -= 1; + } + + /// Tries to remove a upgrade_read waiter from the wait queue, and panics if the + /// waiter is no longer valid. + unsafe fn force_remove_upgrade_read( + &mut self, + wait_node: &mut ListNode, + ) { + if !self.waiters.remove(wait_node) { + // Panic if the address isn't found. This can only happen if the contract was + // violated, e.g. the Entry got moved after the initial poll. + panic!("Future could not be removed from wait queue"); + } + self.nb_waiting_upgrade_reads -= 1; + } + + /// Removes the read from the wait list. + /// + /// This function is only safe as long as the reference that is passed here + /// equals the reference/address under which the waiter was added. + /// The waiter must not have been moved in between. + /// + /// Returns the `Waker` of another task which might get ready to run due to + /// this. + fn remove_read(&mut self, wait_node: &mut ListNode) { + // MutexLockFuture only needs to get removed if it had been added to + // the wait queue of the Mutex. This has happened in the PollState::Waiting case. + // If the current waiter was notified, another waiter must get notified now. + match wait_node.state { + PollState::Notified => { + if self.is_fair { + // In a fair Mutex, the Entry is kept in the + // linked list and must be removed here + // Safety: Due to the state, we know that the node must be part + // of the waiter list + unsafe { self.force_remove_read(wait_node) }; + } + wait_node.state = PollState::Done; + // Since the task was notified but did not lock the Mutex, + // another task gets the chance to run. + self.unlock_read(); + } + PollState::Waiting => { + // Remove the Entry from the linked list + // Safety: Due to the state, we know that the node must be part + // of the waiter list + unsafe { self.force_remove_read(wait_node) }; + wait_node.state = PollState::Done; + } + PollState::New | PollState::Done => (), + } + } + + /// Removes the write from the wait list. + /// + /// This function is only safe as long as the reference that is passed here + /// equals the reference/address under which the waiter was added. + /// The waiter must not have been moved in between. + /// + /// Returns the `Waker` of another task which might get ready to run due to + /// this. + fn remove_write(&mut self, wait_node: &mut ListNode) { + // MutexLockFuture only needs to get removed if it had been added to + // the wait queue of the Mutex. This has happened in the PollState::Waiting case. + // If the current waiter was notified, another waiter must get notified now. + match wait_node.state { + PollState::Notified => { + if self.is_fair { + // In a fair Mutex, the Entry is kept in the + // linked list and must be removed here + // Safety: Due to the state, we know that the node must be part + // of the waiter list + unsafe { self.force_remove_write(wait_node) }; + } + wait_node.state = PollState::Done; + // Since the task was notified but did not lock the Mutex, + // another task gets the chance to run. + self.unlock_write(); + } + PollState::Waiting => { + // Remove the Entry from the linked list + // Safety: Due to the state, we know that the node must be part + // of the waiter list + unsafe { self.force_remove_write(wait_node) }; + wait_node.state = PollState::Done; + } + PollState::New | PollState::Done => (), + } + } + + /// Removes the upgrade_read from the wait list. + /// + /// This function is only safe as long as the reference that is passed here + /// equals the reference/address under which the waiter was added. + /// The waiter must not have been moved in between. + /// + /// Returns the `Waker` of another task which might get ready to run due to + /// this. + fn remove_upgrade_read(&mut self, wait_node: &mut ListNode) { + // MutexLockFuture only needs to get removed if it had been added to + // the wait queue of the Mutex. This has happened in the PollState::Waiting case. + // If the current waiter was notified, another waiter must get notified now. + match wait_node.state { + PollState::Notified => { + if self.is_fair { + // In a fair Mutex, the Entry is kept in the + // linked list and must be removed here + // Safety: Due to the state, we know that the node must be part + // of the waiter list + unsafe { self.force_remove_upgrade_read(wait_node) }; + } + wait_node.state = PollState::Done; + // Since the task was notified but did not lock the Mutex, + // another task gets the chance to run. + self.unlock_upgrade_read(); + } + PollState::Waiting => { + // Remove the Entry from the linked list + // Safety: Due to the state, we know that the node must be part + // of the waiter list + unsafe { self.force_remove_upgrade_read(wait_node) }; + wait_node.state = PollState::Done; + } + PollState::New | PollState::Done => (), + } + } +} + +/// An RAII guard returned by the `read` and `try_read` methods. +/// When this structure is dropped (falls out of scope), the shared +/// read access will be released. +pub struct GenericRwLockReadGuard<'a, MutexType: RawMutex, T: 'a> { + /// The Mutex which is associated with this Guard + mutex: &'a GenericRwLock, +} + +impl core::fmt::Debug + for GenericRwLockReadGuard<'_, MutexType, T> +{ + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("GenericRwLockReadGuard").finish() + } +} + +impl Drop for GenericRwLockReadGuard<'_, MutexType, T> { + fn drop(&mut self) { + // Release the guard. + self.mutex.state.lock().unlock_read(); + } +} + +impl Deref + for GenericRwLockReadGuard<'_, MutexType, T> +{ + type Target = T; + fn deref(&self) -> &T { + unsafe { &*self.mutex.value.get() } + } +} + +// Safety: GenericRwLockReadGuard may only be used across threads if the underlying +// type is Sync. +unsafe impl Sync + for GenericRwLockReadGuard<'_, MutexType, T> +{ +} + +/// A future which resolves when shared read access has been successfully acquired. +#[must_use = "futures do nothing unless polled"] +pub struct GenericRwLockReadFuture<'a, MutexType: RawMutex, T: 'a> { + /// The Mutex which should get locked trough this Future + mutex: Option<&'a GenericRwLock>, + /// Node for waiting at the mutex + wait_node: ListNode, +} + +// Safety: Futures can be sent between threads as long as the underlying +// mutex is thread-safe (Sync), which allows to poll/register/unregister from +// a different thread. +unsafe impl<'a, MutexType: RawMutex + Sync, T: 'a> Send + for GenericRwLockReadFuture<'a, MutexType, T> +{ +} + +impl<'a, MutexType: RawMutex, T: core::fmt::Debug> core::fmt::Debug + for GenericRwLockReadFuture<'a, MutexType, T> +{ + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("GenericRwLockReadFuture").finish() + } +} + +impl<'a, MutexType: RawMutex, T> Future + for GenericRwLockReadFuture<'a, MutexType, T> +{ + type Output = GenericRwLockReadGuard<'a, MutexType, T>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // Safety: The next operations are safe, because Pin promises us that + // the address of the wait queue entry inside GenericRwLockReadFuture is stable, + // and we don't move any fields inside the future until it gets dropped. + let mut_self: &mut GenericRwLockReadFuture = + unsafe { Pin::get_unchecked_mut(self) }; + + let mutex = mut_self + .mutex + .expect("polled GenericRwLockReadFuture after completion"); + let mut mutex_state = mutex.state.lock(); + + let poll_res = + unsafe { mutex_state.try_lock_read(&mut mut_self.wait_node, cx) }; + + match poll_res { + Poll::Pending => Poll::Pending, + Poll::Ready(()) => { + // The mutex was acquired + mut_self.mutex = None; + Poll::Ready(GenericRwLockReadGuard::<'a, MutexType, T> { + mutex, + }) + } + } + } +} + +impl<'a, MutexType: RawMutex, T> FusedFuture + for GenericRwLockReadFuture<'a, MutexType, T> +{ + fn is_terminated(&self) -> bool { + self.mutex.is_none() + } +} + +/// An RAII guard returned by the `write` and `try_write` methods. +/// When this structure is dropped (falls out of scope), the +/// exclusive write access will be released. +pub struct GenericRwLockWriteGuard<'a, MutexType: RawMutex, T: 'a> { + /// The Mutex which is associated with this Guard + mutex: &'a GenericRwLock, +} + +impl core::fmt::Debug + for GenericRwLockWriteGuard<'_, MutexType, T> +{ + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("GenericRwLockWriteGuard").finish() + } +} + +impl Drop + for GenericRwLockWriteGuard<'_, MutexType, T> +{ + fn drop(&mut self) { + // Release the guard. + self.mutex.state.lock().unlock_write(); + } +} + +impl Deref + for GenericRwLockWriteGuard<'_, MutexType, T> +{ + type Target = T; + fn deref(&self) -> &T { + unsafe { &*self.mutex.value.get() } + } +} + +impl DerefMut + for GenericRwLockWriteGuard<'_, MutexType, T> +{ + fn deref_mut(&mut self) -> &mut T { + unsafe { &mut *self.mutex.value.get() } + } +} + +// Safety: GenericRwLockReadGuard may only be used across threads if the underlying +// type is Sync. +unsafe impl Sync + for GenericRwLockWriteGuard<'_, MutexType, T> +{ +} + +/// A future which resolves when exclusive write access has been successfully acquired. +#[must_use = "futures do nothing unless polled"] +pub struct GenericRwLockWriteFuture<'a, MutexType: RawMutex, T: 'a> { + /// The Mutex which should get locked trough this Future + mutex: Option<&'a GenericRwLock>, + /// Node for waiting at the mutex + wait_node: ListNode, +} + +// Safety: Futures can be sent between threads as long as the underlying +// mutex is thread-safe (Sync), which allows to poll/register/unregister from +// a different thread. +unsafe impl<'a, MutexType: RawMutex + Sync, T: 'a> Send + for GenericRwLockWriteFuture<'a, MutexType, T> +{ +} + +impl<'a, MutexType: RawMutex, T: core::fmt::Debug> core::fmt::Debug + for GenericRwLockWriteFuture<'a, MutexType, T> +{ + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("GenericRwLockWriteFuture").finish() + } +} + +impl<'a, MutexType: RawMutex, T> Future + for GenericRwLockWriteFuture<'a, MutexType, T> +{ + type Output = GenericRwLockWriteGuard<'a, MutexType, T>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // Safety: The next operations are safe, because Pin promises us that + // the address of the wait queue entry inside GenericRwLockReadFuture is stable, + // and we don't move any fields inside the future until it gets dropped. + let mut_self: &mut GenericRwLockWriteFuture = + unsafe { Pin::get_unchecked_mut(self) }; + + let mutex = mut_self + .mutex + .expect("polled GenericRwLockWriteFuture after completion"); + let mut mutex_state = mutex.state.lock(); + + let poll_res = + unsafe { mutex_state.try_lock_write(&mut mut_self.wait_node, cx) }; + + match poll_res { + Poll::Pending => Poll::Pending, + Poll::Ready(()) => { + // The mutex was acquired + mut_self.mutex = None; + Poll::Ready(GenericRwLockWriteGuard::<'a, MutexType, T> { + mutex, + }) + } + } + } +} + +impl<'a, MutexType: RawMutex, T> FusedFuture + for GenericRwLockWriteFuture<'a, MutexType, T> +{ + fn is_terminated(&self) -> bool { + self.mutex.is_none() + } +} + +impl<'a, MutexType: RawMutex, T> Drop + for GenericRwLockWriteFuture<'a, MutexType, T> +{ + fn drop(&mut self) { + // If this GenericRwLockReadFuture has been polled and it was added to the + // wait queue at the mutex, it must be removed before dropping. + // Otherwise the mutex would access invalid memory. + if let Some(mutex) = self.mutex { + let mut mutex_state = mutex.state.lock(); + mutex_state.remove_write(&mut self.wait_node) + } + } +} + +/// An RAII guard returned by the `write` and `try_write` methods. +/// When this structure is dropped (falls out of scope), the +/// exclusive write access will be released. +pub struct GenericRwLockUpgradableReadGuard<'a, MutexType: RawMutex, T: 'a> { + /// The Mutex which is associated with this Guard + mutex: Option<&'a GenericRwLock>, +} + +impl<'a, MutexType: RawMutex, T> GenericRwLockUpgradableReadGuard<'a, MutexType, T> { + /// Asynchrousnly upgrade the shared read lock into an exclusive write lock. + pub async fn upgrade(mut self) -> GenericRwLockWriteFuture<'a, MutexType, T> { + let mutex = self.mutex.take().unwrap(); + let mut state = mutex.state.lock(); + state.unlock_upgrade_write(); + + GenericRwLockWriteFuture:: { + mutex: Some(mutex), + wait_node: ListNode::new(Entry::new(EntryKind::Write)), + } + } + + /// Atomically upgrade the shared read lock into an exclusive write lock, + /// blocking the current thread. + pub async fn try_upgrade(mut self) -> Result, Self> { + let mutex = self.mutex.take().unwrap(); + let mut state = mutex.state.lock(); + if state.try_upgrade_write_sync() { + Ok(GenericRwLockWriteGuard { mutex }) + } else { + Err(Self { mutex: Some(mutex) }) + } + } +} + +impl core::fmt::Debug + for GenericRwLockUpgradableReadGuard<'_, MutexType, T> +{ + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("GenericRwLockUpgradableReadGuard").finish() + } +} + +impl Drop + for GenericRwLockUpgradableReadGuard<'_, MutexType, T> +{ + fn drop(&mut self) { + if let Some(mutex) = self.mutex.take() { + // Release the guard. + mutex.state.lock().unlock_upgrade_read(); + } + } +} + +impl Deref + for GenericRwLockUpgradableReadGuard<'_, MutexType, T> +{ + type Target = T; + fn deref(&self) -> &T { + unsafe { &*self.mutex.as_ref().unwrap().value.get() } + } +} + +// Safety: GenericRwLockReadGuard may only be used across threads if the underlying +// type is Sync. +unsafe impl Sync + for GenericRwLockUpgradableReadGuard<'_, MutexType, T> +{ +} + + +/// A future which resolves when exclusive write access has been successfully acquired. +#[must_use = "futures do nothing unless polled"] +pub struct GenericRwLockUpgradableReadFuture<'a, MutexType: RawMutex, T: 'a> { + /// The Mutex which should get locked trough this Future + mutex: Option<&'a GenericRwLock>, + /// Node for waiting at the mutex + wait_node: ListNode, +} + +// Safety: Futures can be sent between threads as long as the underlying +// mutex is thread-safe (Sync), which allows to poll/register/unregister from +// a different thread. +unsafe impl<'a, MutexType: RawMutex + Sync, T: 'a> Send + for GenericRwLockUpgradableReadFuture<'a, MutexType, T> +{ +} + +impl<'a, MutexType: RawMutex, T: core::fmt::Debug> core::fmt::Debug + for GenericRwLockUpgradableReadFuture<'a, MutexType, T> +{ + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("GenericRwLockUpgradableReadFuture").finish() + } +} + +impl<'a, MutexType: RawMutex, T> Future + for GenericRwLockUpgradableReadFuture<'a, MutexType, T> +{ + type Output = GenericRwLockUpgradableReadGuard<'a, MutexType, T>; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // Safety: The next operations are safe, because Pin promises us that + // the address of the wait queue entry inside GenericRwLockReadFuture is stable, + // and we don't move any fields inside the future until it gets dropped. + let mut_self: &mut GenericRwLockUpgradableReadFuture = + unsafe { Pin::get_unchecked_mut(self) }; + + let mutex = mut_self + .mutex + .expect("polled GenericRwLockUpgradableReadFuture after completion"); + let mut mutex_state = mutex.state.lock(); + + let poll_res = + unsafe { mutex_state.try_lock_upgrade_read(&mut mut_self.wait_node, cx) }; + + match poll_res { + Poll::Pending => Poll::Pending, + Poll::Ready(()) => { + // The mutex was acquired + mut_self.mutex = None; + Poll::Ready(GenericRwLockUpgradableReadGuard::<'a, MutexType, T> { + mutex: Some(mutex), + }) + } + } + } +} + +impl<'a, MutexType: RawMutex, T> FusedFuture + for GenericRwLockUpgradableReadFuture<'a, MutexType, T> +{ + fn is_terminated(&self) -> bool { + self.mutex.is_none() + } +} + +impl<'a, MutexType: RawMutex, T> Drop + for GenericRwLockUpgradableReadFuture<'a, MutexType, T> +{ + fn drop(&mut self) { + // If this GenericRwLockReadFuture has been polled and it was added to the + // wait queue at the mutex, it must be removed before dropping. + // Otherwise the mutex would access invalid memory. + if let Some(mutex) = self.mutex { + let mut mutex_state = mutex.state.lock(); + mutex_state.remove_upgrade_read(&mut self.wait_node) + } + } +} +/// A futures-aware mutex. +pub struct GenericRwLock { + value: UnsafeCell, + state: LockApiMutex, +} + +// It is safe to send mutexes between threads, as long as they are not used and +// thereby borrowed +unsafe impl Send + for GenericRwLock +{ +} +// The mutex is thread-safe as long as the utilized mutex is thread-safe +unsafe impl Sync + for GenericRwLock +{ +} + +impl core::fmt::Debug + for GenericRwLock +{ + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + f.debug_struct("Mutex") + .field("is_exclusive", &self.is_exclusive()) + .finish() + } +} + +impl GenericRwLock { + /// Creates a new futures-aware mutex. + /// + /// `is_fair` defines whether the `Mutex` should behave be fair regarding the + /// order of waiters. A fair `Mutex` will only allow the first waiter which + /// tried to lock but failed to lock the `Mutex` once it's available again. + /// Other waiters must wait until either this locking attempt completes, and + /// the `Mutex` gets unlocked again, or until the `MutexLockFuture` which + /// tried to gain the lock is dropped. + pub fn new(value: T, is_fair: bool) -> GenericRwLock { + GenericRwLock:: { + value: UnsafeCell::new(value), + state: LockApiMutex::new(MutexState::new(is_fair)), + } + } + + /// Acquire shared read access asynchronously. + /// + /// This method returns a future that will resolve once the shared + /// read access has been successfully acquired. + pub fn read(&self) -> GenericRwLockReadFuture<'_, MutexType, T> { + GenericRwLockReadFuture:: { + mutex: Some(&self), + wait_node: ListNode::new(Entry::new(EntryKind::Read)), + } + } + + /// Tries to acquire shared read access without waiting. + /// + /// If acquiring the mutex is successful, a [`GenericRwLockReadGuard`] + /// will be returned, which allows to access the contained data. + /// + /// Otherwise `None` will be returned. + pub fn try_read(&self) -> Option> { + if self.state.lock().try_lock_read_sync() { + Some(GenericRwLockReadGuard { mutex: self }) + } else { + None + } + } + + /// Acquire exclusive write access asynchronously. + /// + /// This method returns a future that will resolve once the exclusive + /// write access has been successfully acquired. + pub fn write(&self) -> GenericRwLockWriteFuture<'_, MutexType, T> { + GenericRwLockWriteFuture:: { + mutex: Some(&self), + wait_node: ListNode::new(Entry::new(EntryKind::Write)), + } + } + + /// Tries to acquire exclusive write access without waiting. + /// + /// If acquiring the mutex is successful, a [`GenericRwLockReadGuard`] + /// will be returned, which allows to access the contained data. + /// + /// Otherwise `None` will be returned. + pub fn try_write( + &self, + ) -> Option> { + if self.state.lock().try_lock_write_sync() { + Some(GenericRwLockWriteGuard { mutex: self }) + } else { + None + } + } + + + /// Acquire upgradable shared read access asynchronously. + /// + /// This method returns a future that will resolve once the upgradable + /// shared read access has been successfully acquired. + pub fn upgradable_read(&self) -> GenericRwLockUpgradableReadFuture<'_, MutexType, T> { + GenericRwLockUpgradableReadFuture:: { + mutex: Some(&self), + wait_node: ListNode::new(Entry::new(EntryKind::UpgradeRead)), + } + } + + /// Tries to acquire exclusive write access without waiting. + /// + /// If acquiring the mutex is successful, a [`GenericRwLockReadGuard`] + /// will be returned, which allows to access the contained data. + /// + /// Otherwise `None` will be returned. + pub fn try_upgradable_read( + &self, + ) -> Option> { + if self.state.lock().try_lock_upgrade_read_sync() { + Some(GenericRwLockUpgradableReadGuard { mutex: Some(self) }) + } else { + None + } + } + + /// Returns whether the rwlock is locked in exclusive access. + pub fn is_exclusive(&self) -> bool { + self.state.lock().has_write + } +} + +// Export a non thread-safe version using NoopLock + +/// A [`GenericRwLock`] which is not thread-safe. +pub type LocalRwLock = GenericRwLock; + +/// A [`GenericRwLockReadGuard`] for [`LocalMutex`]. +pub type LocalRwLockReadGuard<'a, T> = GenericRwLockReadGuard<'a, NoopLock, T>; + +/// A [`GenericRwLockReadFuture`] for [`LocalMutex`]. +pub type LocalRwLockReadFuture<'a, T> = + GenericRwLockReadFuture<'a, NoopLock, T>; + +/// A [`GenericRwLockWriteGuard`] for [`LocalMutex`]. +pub type LocalRwLockWriteGuard<'a, T> = GenericRwLockWriteGuard<'a, NoopLock, T>; + +/// A [`GenericRwLockWriteFuture`] for [`LocalMutex`]. +pub type LocalRwLockWriteFuture<'a, T> = + GenericRwLockWriteFuture<'a, NoopLock, T>; + +/// A [`GenericRwLockUpgradableReadGuard`] for [`LocalMutex`]. +pub type LocalRwLockUpgradableReadGuard<'a, T> = GenericRwLockUpgradableReadGuard<'a, NoopLock, T>; + +/// A [`GenericRwLockUpgradableReadFuture`] for [`LocalMutex`]. +pub type LocalRwLockUpgradableReadFuture<'a, T> = + GenericRwLockUpgradableReadFuture<'a, NoopLock, T>; + +#[cfg(feature = "std")] +mod if_std { + use super::*; + + // Export a thread-safe version using parking_lot::RawMutex + + /// A [`GenericRwLock`] backed by [`parking_lot`]. + pub type RwLock = GenericRwLock; + + /// A [`GenericRwLockReadGuard`] for [`Mutex`]. + pub type RwLockReadGuard<'a, T> = + GenericRwLockReadGuard<'a, parking_lot::RawMutex, T>; + + /// A [`GenericRwLockReadFuture`] for [`Mutex`]. + pub type RwLockReadFuture<'a, T> = + GenericRwLockReadFuture<'a, parking_lot::RawMutex, T>; + + /// A [`GenericRwLockWriteGuard`] for [`Mutex`]. + pub type RwLockWriteGuard<'a, T> = + GenericRwLockWriteGuard<'a, parking_lot::RawMutex, T>; + + /// A [`GenericRwLockWriteFuture`] for [`Mutex`]. + pub type RwLockWriteFuture<'a, T> = + GenericRwLockWriteFuture<'a, parking_lot::RawMutex, T>; + + /// A [`GenericRwLockUpgradableReadGuard`] for [`Mutex`]. + pub type RwLockUpgradableReadGuard<'a, T> = + GenericRwLockUpgradableReadGuard<'a, parking_lot::RawMutex, T>; + + /// A [`GenericRwLockUpgradableReadFuture`] for [`Mutex`]. + pub type RwLockUpgradableReadFuture<'a, T> = + GenericRwLockUpgradableReadFuture<'a, parking_lot::RawMutex, T>; +} + +#[cfg(feature = "std")] +pub use self::if_std::*; From b2b35b1900afcf1f9c945e266eccec3dd3c08faa Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 04:16:16 -0400 Subject: [PATCH 02/27] Fix imports & guard drop impl --- src/sync/mod.rs | 15 ++++++ src/sync/rwlock.rs | 112 +++++++++++++++++++++++++++------------------ 2 files changed, 83 insertions(+), 44 deletions(-) diff --git a/src/sync/mod.rs b/src/sync/mod.rs index 54b7f28..63b8789 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -43,3 +43,18 @@ pub use self::semaphore::{ }; mod rwlock; + +pub use self::rwlock::{ + GenericRwLock, GenericRwLockReadFuture, GenericRwLockReadGuard, + GenericRwLockUpgradableReadFuture, GenericRwLockUpgradableReadGuard, + GenericRwLockWriteFuture, GenericRwLockWriteGuard, LocalRwLock, + LocalRwLockReadFuture, LocalRwLockReadGuard, + LocalRwLockUpgradableReadFuture, LocalRwLockUpgradableReadGuard, + LocalRwLockWriteFuture, LocalRwLockWriteGuard, +}; + +#[cfg(feature = "std")] +pub use self::rwlock::{ + RwLock, RwLockReadFuture, RwLockReadGuard, RwLockUpgradableReadFuture, + RwLockUpgradableReadGuard, RwLockWriteFuture, RwLockWriteGuard, +}; diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index dd2d3a4..0c7b814 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -179,38 +179,34 @@ impl MutexState { fn wakeup_any_waiters(&mut self) { let mut nb_reads = 0; if self.is_fair { - self.waiters.reverse_apply_while(|entry| { - match entry.kind { - EntryKind::Read | EntryKind::UpgradeRead => { + self.waiters.reverse_apply_while(|entry| match entry.kind { + EntryKind::Read | EntryKind::UpgradeRead => { + entry.notify(); + nb_reads += 1; + ControlFlow::Continue + } + EntryKind::Write => { + if nb_reads == 0 { entry.notify(); - nb_reads += 1; + ControlFlow::Stop + } else { ControlFlow::Continue } - EntryKind::Write => { - if nb_reads == 0 { - entry.notify(); - ControlFlow::Stop - } else { - ControlFlow::Continue - } - } } }); } else { - self.waiters.reverse_apply_while(|entry| { - match entry.kind { - EntryKind::Read | EntryKind::UpgradeRead => { + self.waiters.reverse_apply_while(|entry| match entry.kind { + EntryKind::Read | EntryKind::UpgradeRead => { + entry.notify(); + nb_reads += 1; + ControlFlow::RemoveAndContinue + } + EntryKind::Write => { + if nb_reads == 0 { entry.notify(); - nb_reads += 1; - ControlFlow::RemoveAndContinue - } - EntryKind::Write => { - if nb_reads == 0 { - entry.notify(); - ControlFlow::RemoveAndStop - } else { - ControlFlow::Continue - } + ControlFlow::RemoveAndStop + } else { + ControlFlow::Continue } } }); @@ -818,7 +814,7 @@ impl<'a, MutexType: RawMutex, T> Future fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { // Safety: The next operations are safe, because Pin promises us that - // the address of the wait queue entry inside GenericRwLockReadFuture is stable, + // the address of the wait queue entry inside this future is stable, // and we don't move any fields inside the future until it gets dropped. let mut_self: &mut GenericRwLockReadFuture = unsafe { Pin::get_unchecked_mut(self) }; @@ -852,6 +848,20 @@ impl<'a, MutexType: RawMutex, T> FusedFuture } } +impl<'a, MutexType: RawMutex, T> Drop + for GenericRwLockReadFuture<'a, MutexType, T> +{ + fn drop(&mut self) { + // If this GenericRwLockReadFuture has been polled and it was added to the + // wait queue at the mutex, it must be removed before dropping. + // Otherwise the mutex would access invalid memory. + if let Some(mutex) = self.mutex { + let mut mutex_state = mutex.state.lock(); + mutex_state.remove_read(&mut self.wait_node) + } + } +} + /// An RAII guard returned by the `write` and `try_write` methods. /// When this structure is dropped (falls out of scope), the /// exclusive write access will be released. @@ -933,7 +943,7 @@ impl<'a, MutexType: RawMutex, T> Future fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { // Safety: The next operations are safe, because Pin promises us that - // the address of the wait queue entry inside GenericRwLockReadFuture is stable, + // the address of the wait queue entry inside this future is stable, // and we don't move any fields inside the future until it gets dropped. let mut_self: &mut GenericRwLockWriteFuture = unsafe { Pin::get_unchecked_mut(self) }; @@ -971,7 +981,7 @@ impl<'a, MutexType: RawMutex, T> Drop for GenericRwLockWriteFuture<'a, MutexType, T> { fn drop(&mut self) { - // If this GenericRwLockReadFuture has been polled and it was added to the + // If this GenericRwLockWriteFuture has been polled and it was added to the // wait queue at the mutex, it must be removed before dropping. // Otherwise the mutex would access invalid memory. if let Some(mutex) = self.mutex { @@ -989,9 +999,13 @@ pub struct GenericRwLockUpgradableReadGuard<'a, MutexType: RawMutex, T: 'a> { mutex: Option<&'a GenericRwLock>, } -impl<'a, MutexType: RawMutex, T> GenericRwLockUpgradableReadGuard<'a, MutexType, T> { +impl<'a, MutexType: RawMutex, T> + GenericRwLockUpgradableReadGuard<'a, MutexType, T> +{ /// Asynchrousnly upgrade the shared read lock into an exclusive write lock. - pub async fn upgrade(mut self) -> GenericRwLockWriteFuture<'a, MutexType, T> { + pub async fn upgrade( + mut self, + ) -> GenericRwLockWriteFuture<'a, MutexType, T> { let mutex = self.mutex.take().unwrap(); let mut state = mutex.state.lock(); state.unlock_upgrade_write(); @@ -1004,7 +1018,9 @@ impl<'a, MutexType: RawMutex, T> GenericRwLockUpgradableReadGuard<'a, MutexType, /// Atomically upgrade the shared read lock into an exclusive write lock, /// blocking the current thread. - pub async fn try_upgrade(mut self) -> Result, Self> { + pub async fn try_upgrade( + mut self, + ) -> Result, Self> { let mutex = self.mutex.take().unwrap(); let mut state = mutex.state.lock(); if state.try_upgrade_write_sync() { @@ -1050,7 +1066,6 @@ unsafe impl Sync { } - /// A future which resolves when exclusive write access has been successfully acquired. #[must_use = "futures do nothing unless polled"] pub struct GenericRwLockUpgradableReadFuture<'a, MutexType: RawMutex, T: 'a> { @@ -1083,25 +1098,30 @@ impl<'a, MutexType: RawMutex, T> Future fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { // Safety: The next operations are safe, because Pin promises us that - // the address of the wait queue entry inside GenericRwLockReadFuture is stable, + // the address of the wait queue entry inside the future is stable, // and we don't move any fields inside the future until it gets dropped. let mut_self: &mut GenericRwLockUpgradableReadFuture = unsafe { Pin::get_unchecked_mut(self) }; - let mutex = mut_self - .mutex - .expect("polled GenericRwLockUpgradableReadFuture after completion"); + let mutex = mut_self.mutex.expect( + "polled GenericRwLockUpgradableReadFuture after completion", + ); let mut mutex_state = mutex.state.lock(); - let poll_res = - unsafe { mutex_state.try_lock_upgrade_read(&mut mut_self.wait_node, cx) }; + let poll_res = unsafe { + mutex_state.try_lock_upgrade_read(&mut mut_self.wait_node, cx) + }; match poll_res { Poll::Pending => Poll::Pending, Poll::Ready(()) => { // The mutex was acquired mut_self.mutex = None; - Poll::Ready(GenericRwLockUpgradableReadGuard::<'a, MutexType, T> { + Poll::Ready(GenericRwLockUpgradableReadGuard::< + 'a, + MutexType, + T, + > { mutex: Some(mutex), }) } @@ -1121,7 +1141,7 @@ impl<'a, MutexType: RawMutex, T> Drop for GenericRwLockUpgradableReadFuture<'a, MutexType, T> { fn drop(&mut self) { - // If this GenericRwLockReadFuture has been polled and it was added to the + // If this future has been polled and it was added to the // wait queue at the mutex, it must be removed before dropping. // Otherwise the mutex would access invalid memory. if let Some(mutex) = self.mutex { @@ -1130,6 +1150,7 @@ impl<'a, MutexType: RawMutex, T> Drop } } } + /// A futures-aware mutex. pub struct GenericRwLock { value: UnsafeCell, @@ -1226,12 +1247,13 @@ impl GenericRwLock { } } - /// Acquire upgradable shared read access asynchronously. /// /// This method returns a future that will resolve once the upgradable /// shared read access has been successfully acquired. - pub fn upgradable_read(&self) -> GenericRwLockUpgradableReadFuture<'_, MutexType, T> { + pub fn upgradable_read( + &self, + ) -> GenericRwLockUpgradableReadFuture<'_, MutexType, T> { GenericRwLockUpgradableReadFuture:: { mutex: Some(&self), wait_node: ListNode::new(Entry::new(EntryKind::UpgradeRead)), @@ -1273,14 +1295,16 @@ pub type LocalRwLockReadFuture<'a, T> = GenericRwLockReadFuture<'a, NoopLock, T>; /// A [`GenericRwLockWriteGuard`] for [`LocalMutex`]. -pub type LocalRwLockWriteGuard<'a, T> = GenericRwLockWriteGuard<'a, NoopLock, T>; +pub type LocalRwLockWriteGuard<'a, T> = + GenericRwLockWriteGuard<'a, NoopLock, T>; /// A [`GenericRwLockWriteFuture`] for [`LocalMutex`]. pub type LocalRwLockWriteFuture<'a, T> = GenericRwLockWriteFuture<'a, NoopLock, T>; /// A [`GenericRwLockUpgradableReadGuard`] for [`LocalMutex`]. -pub type LocalRwLockUpgradableReadGuard<'a, T> = GenericRwLockUpgradableReadGuard<'a, NoopLock, T>; +pub type LocalRwLockUpgradableReadGuard<'a, T> = + GenericRwLockUpgradableReadGuard<'a, NoopLock, T>; /// A [`GenericRwLockUpgradableReadFuture`] for [`LocalMutex`]. pub type LocalRwLockUpgradableReadFuture<'a, T> = From d225aa4d2db680e3478a124406c6e8f7a2c35caf Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 04:41:07 -0400 Subject: [PATCH 03/27] Add some tests --- src/sync/rwlock.rs | 5 ++ tests/rwlock.rs | 124 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 tests/rwlock.rs diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 0c7b814..fc192ea 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -1280,6 +1280,11 @@ impl GenericRwLock { pub fn is_exclusive(&self) -> bool { self.state.lock().has_write } + + /// Returns the number of shared read access guards currently held. + pub fn nb_readers(&self) -> usize { + self.state.lock().nb_reads + } } // Export a non thread-safe version using NoopLock diff --git a/tests/rwlock.rs b/tests/rwlock.rs new file mode 100644 index 0000000..f826bbd --- /dev/null +++ b/tests/rwlock.rs @@ -0,0 +1,124 @@ +use futures::future::{FusedFuture, Future}; +use futures::task::{Context, Poll}; +use futures_intrusive::sync::LocalRwLock; +use futures_test::task::{new_count_waker, panic_waker}; +use pin_utils::pin_mut; + +macro_rules! gen_rwlock_tests { + ($mod_name:ident, $rwlock_type:ident) => { + mod $mod_name { + use super::*; + + #[test] + fn uncontended_read() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + { + let mut guards = Vec::with_capacity(3); + for _ in 0..3 { + let fut = lock.read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => { + panic!("Expect mutex to get locked") + } + Poll::Ready(mut guard) => { + assert_eq!(false, lock.is_exclusive()); + assert_eq!(5, *guard); + guards.push(guard); + assert_eq!(guards.len(), lock.nb_readers()); + } + }; + assert!(fut.as_mut().is_terminated()); + } + + assert_eq!(3, lock.nb_readers()); + + drop(guards.pop().unwrap()); + assert_eq!(2, lock.nb_readers()); + + drop(guards.pop().unwrap()); + assert_eq!(1, lock.nb_readers()); + + drop(guards.pop().unwrap()); + assert_eq!(0, lock.nb_readers()); + } + + { + let fut = lock.read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => { + panic!("Expect mutex to get locked") + } + Poll::Ready(guard) => { + assert_eq!(false, lock.is_exclusive()); + assert_eq!(1, lock.nb_readers()); + assert_eq!(5, *guard); + } + }; + } + + assert_eq!(0, lock.nb_readers()); + } + } + } + }; +} + +gen_rwlock_tests!(local_rwlock_tests, LocalRwLock); + +#[cfg(feature = "std")] +mod if_std { + use super::*; + use futures::FutureExt; + use futures_intrusive::sync::RwLock; + + gen_rwlock_tests!(mutex_tests, RwLock); + + fn is_send(_: &T) {} + + fn is_send_value(_: T) {} + + fn is_sync(_: &T) {} + + macro_rules! gen_future_send_test { + ($mod_name:ident, $fut_fn:ident) => { + mod $mod_name { + use super::*; + + #[test] + fn futures_are_send() { + let lock = RwLock::new(true, true); + is_sync(&lock); + { + let fut = lock.$fut_fn(); + is_send(&fut); + pin_mut!(fut); + is_send(&fut); + + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + pin_mut!(fut); + let res = fut.poll_unpin(cx); + let guard = match res { + Poll::Ready(v) => v, + Poll::Pending => panic!("Expected to be ready"), + }; + is_send(&guard); + is_send_value(guard); + } + is_send_value(lock); + } + } + }; + } + + gen_future_send_test!(rwlock_read, read); + gen_future_send_test!(rwlock_write, write); + gen_future_send_test!(rwlock_upgradable_read, upgradable_read); +} From 37252b1dd323b9ffc4ba9ee6e171ff630af295ab Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 04:44:55 -0400 Subject: [PATCH 04/27] Add test --- tests/rwlock.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index f826bbd..bef7947 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -66,6 +66,50 @@ macro_rules! gen_rwlock_tests { assert_eq!(0, lock.nb_readers()); } } + + #[test] + fn uncontended_write() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + { + let fut = lock.write(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => { + panic!("Expect mutex to get locked") + } + Poll::Ready(mut guard) => { + assert_eq!(true, lock.is_exclusive()); + assert_eq!(5, *guard); + *guard = 12; + assert_eq!(12, *guard); + } + }; + assert!(fut.as_mut().is_terminated()); + } + assert_eq!(false, lock.is_exclusive()); + + { + let fut = lock.write(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => { + panic!("Expect mutex to get locked") + } + Poll::Ready(guard) => { + assert_eq!(true, lock.is_exclusive()); + assert_eq!(12, *guard); + } + }; + } + + assert_eq!(false, lock.is_exclusive()); + } + } } }; } From 94784d3fe819d1f3dfe6cdf328d413836617f8fa Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 04:49:13 -0400 Subject: [PATCH 05/27] Add test --- src/sync/rwlock.rs | 6 ++--- tests/rwlock.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index fc192ea..f7e646a 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -1003,9 +1003,7 @@ impl<'a, MutexType: RawMutex, T> GenericRwLockUpgradableReadGuard<'a, MutexType, T> { /// Asynchrousnly upgrade the shared read lock into an exclusive write lock. - pub async fn upgrade( - mut self, - ) -> GenericRwLockWriteFuture<'a, MutexType, T> { + pub fn upgrade(mut self) -> GenericRwLockWriteFuture<'a, MutexType, T> { let mutex = self.mutex.take().unwrap(); let mut state = mutex.state.lock(); state.unlock_upgrade_write(); @@ -1018,7 +1016,7 @@ impl<'a, MutexType: RawMutex, T> /// Atomically upgrade the shared read lock into an exclusive write lock, /// blocking the current thread. - pub async fn try_upgrade( + pub fn try_upgrade( mut self, ) -> Result, Self> { let mutex = self.mutex.take().unwrap(); diff --git a/tests/rwlock.rs b/tests/rwlock.rs index bef7947..e60b3f2 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -110,6 +110,64 @@ macro_rules! gen_rwlock_tests { assert_eq!(false, lock.is_exclusive()); } } + + #[test] + fn uncontended_upgradable_read() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + { + let fut = lock.upgradable_read(); + pin_mut!(fut); + let guard = match fut.as_mut().poll(cx) { + Poll::Pending => { + panic!("Expect mutex to get locked") + } + Poll::Ready(mut guard) => { + assert_eq!(false, lock.is_exclusive()); + assert_eq!(5, *guard); + guard + } + }; + assert!(fut.as_mut().is_terminated()); + + let fut = guard.upgrade(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => { + panic!("Expect mutex to get locked") + } + Poll::Ready(mut guard) => { + assert_eq!(true, lock.is_exclusive()); + assert_eq!(5, *guard); + *guard = 12; + assert_eq!(12, *guard); + } + }; + } + + assert_eq!(false, lock.is_exclusive()); + + { + let fut = lock.upgradable_read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => { + panic!("Expect mutex to get locked") + } + Poll::Ready(guard) => { + assert_eq!(false, lock.is_exclusive()); + assert_eq!(12, *guard); + } + }; + } + + assert_eq!(false, lock.is_exclusive()); + } + } } }; } From f2df4ae81c35d3c8c8e191a2ad01fe9874dc0880 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 04:53:32 -0400 Subject: [PATCH 06/27] Rename confusing fn --- src/sync/rwlock.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index f7e646a..24f6055 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -167,7 +167,7 @@ impl MutexState { /// it into a writer lock, waking up entries if needed. /// /// If the Mutex is not fair does not wakeup anyone. - fn unlock_upgrade_write(&mut self) { + fn unlock_upgrade_read_for_upgrade(&mut self) { self.has_upgrade_read = false; self.nb_reads -= 1; @@ -274,7 +274,7 @@ impl MutexState { /// Attempt to gain exclusive write access. /// /// Returns true if the access is obtained. - fn try_upgrade_write_sync(&mut self) -> bool { + fn try_upgrade_read_sync(&mut self) -> bool { // The lock can only be obtained synchronously if // - has no write // - has 1 read (the caller) @@ -1006,7 +1006,7 @@ impl<'a, MutexType: RawMutex, T> pub fn upgrade(mut self) -> GenericRwLockWriteFuture<'a, MutexType, T> { let mutex = self.mutex.take().unwrap(); let mut state = mutex.state.lock(); - state.unlock_upgrade_write(); + state.unlock_upgrade_read_for_upgrade(); GenericRwLockWriteFuture:: { mutex: Some(mutex), @@ -1021,7 +1021,7 @@ impl<'a, MutexType: RawMutex, T> ) -> Result, Self> { let mutex = self.mutex.take().unwrap(); let mut state = mutex.state.lock(); - if state.try_upgrade_write_sync() { + if state.try_upgrade_read_sync() { Ok(GenericRwLockWriteGuard { mutex }) } else { Err(Self { mutex: Some(mutex) }) From 08342a5de3c466d50b6f6dd3fb3eb56d33204f92 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 05:03:33 -0400 Subject: [PATCH 07/27] Add unsafe doc --- src/intrusive_double_linked_list.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/intrusive_double_linked_list.rs b/src/intrusive_double_linked_list.rs index beaf875..31dd786 100644 --- a/src/intrusive_double_linked_list.rs +++ b/src/intrusive_double_linked_list.rs @@ -331,6 +331,10 @@ impl LinkedList { let mut current = self.tail; while let Some(mut node) = current { + // Safety: We are exclusively using nodes that are already contained + // in the list so they will contain valid data. The nodes can also + // not be added to the list again during iteration, since the list + // is mutably borrowed. unsafe { let flow = func(node.as_mut()); match flow { From b8a2ec6ef4fce309489e54f64d6df927f1af3861 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 19:36:13 -0400 Subject: [PATCH 08/27] Add contention tests --- src/sync/rwlock.rs | 3 +- tests/rwlock.rs | 77 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 24f6055..41a1ccf 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -325,7 +325,7 @@ impl MutexState { /// In addition to this `node` may not be added to another other list before /// it is removed from the current one. unsafe fn add_upgrade_read(&mut self, wait_node: &mut ListNode) { - debug_assert_eq!(wait_node.kind, EntryKind::Write); + debug_assert_eq!(wait_node.kind, EntryKind::UpgradeRead); self.waiters.add_front(wait_node); self.nb_waiting_upgrade_reads += 1; } @@ -352,6 +352,7 @@ impl MutexState { // Add the task to the wait queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; + self.add_read(wait_node); Poll::Pending } } diff --git a/tests/rwlock.rs b/tests/rwlock.rs index e60b3f2..1406f1b 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -1,9 +1,13 @@ +#![feature(collapse_debuginfo)] + use futures::future::{FusedFuture, Future}; use futures::task::{Context, Poll}; use futures_intrusive::sync::LocalRwLock; use futures_test::task::{new_count_waker, panic_waker}; use pin_utils::pin_mut; +// Allows backtrace to work properly. +#[collapse_debuginfo] macro_rules! gen_rwlock_tests { ($mod_name:ident, $rwlock_type:ident) => { mod $mod_name { @@ -24,7 +28,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect mutex to get locked") + panic!("Expect lock to get locked") } Poll::Ready(mut guard) => { assert_eq!(false, lock.is_exclusive()); @@ -53,7 +57,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect mutex to get locked") + panic!("Expect lock to get locked") } Poll::Ready(guard) => { assert_eq!(false, lock.is_exclusive()); @@ -80,7 +84,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect mutex to get locked") + panic!("Expect lock to get locked") } Poll::Ready(mut guard) => { assert_eq!(true, lock.is_exclusive()); @@ -98,7 +102,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect mutex to get locked") + panic!("Expect lock to get locked") } Poll::Ready(guard) => { assert_eq!(true, lock.is_exclusive()); @@ -124,7 +128,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); let guard = match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect mutex to get locked") + panic!("Expect lock to get locked") } Poll::Ready(mut guard) => { assert_eq!(false, lock.is_exclusive()); @@ -138,7 +142,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect mutex to get locked") + panic!("Expect lock to get locked") } Poll::Ready(mut guard) => { assert_eq!(true, lock.is_exclusive()); @@ -156,7 +160,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect mutex to get locked") + panic!("Expect lock to get locked") } Poll::Ready(guard) => { assert_eq!(false, lock.is_exclusive()); @@ -168,6 +172,63 @@ macro_rules! gen_rwlock_tests { assert_eq!(false, lock.is_exclusive()); } } + + #[test] + fn contended_read() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + let guard = lock.try_write().unwrap(); + + { + assert!(lock.try_read().is_none()); + let fut = lock.read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut.as_mut().is_terminated()); + } + + { + assert!(lock.try_upgradable_read().is_none()); + let fut = lock.upgradable_read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut.as_mut().is_terminated()); + } + } + } + + #[test] + fn contended_write() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + let guard = lock.try_read().unwrap(); + + { + assert!(lock.try_write().is_none()); + let fut = lock.write(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut.as_mut().is_terminated()); + } + } + } } }; } @@ -180,7 +241,7 @@ mod if_std { use futures::FutureExt; use futures_intrusive::sync::RwLock; - gen_rwlock_tests!(mutex_tests, RwLock); + gen_rwlock_tests!(rwlock_tests, RwLock); fn is_send(_: &T) {} From e5518c06112c825d6ea44f3e7e1d92e78227bbcb Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 19:48:16 -0400 Subject: [PATCH 09/27] Add basic wakeup tests --- tests/rwlock.rs | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index 1406f1b..67c0e70 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -229,6 +229,82 @@ macro_rules! gen_rwlock_tests { } } } + + #[test] + fn dropping_reader_wakes_up_writer() { + for is_fair in &[true, false] { + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + let guard = lock.try_read().unwrap(); + + let mut fut = lock.write(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut.as_mut().is_terminated()); + + assert_eq!(count, 0); + + drop(guard); + assert_eq!(count, 1); + + match fut.as_mut().poll(cx) { + Poll::Pending => panic!("rwlock busy"), + Poll::Ready(mut guard) => (), + }; + assert!(fut.as_mut().is_terminated()); + } + } + + #[test] + fn dropping_writer_wakes_up_readers() { + for is_fair in &[true, false] { + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + let guard = lock.try_write().unwrap(); + + let fut1 = lock.read(); + pin_mut!(fut1); + match fut1.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut1.as_mut().is_terminated()); + + let fut2 = lock.upgradable_read(); + pin_mut!(fut2); + match fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut2.as_mut().is_terminated()); + + assert_eq!(count, 0); + + drop(guard); + assert_eq!(count, 2); + + match fut1.as_mut().poll(cx) { + Poll::Pending => panic!("rwlock busy"), + Poll::Ready(mut guard) => (), + }; + assert!(fut1.as_mut().is_terminated()); + + match fut2.as_mut().poll(cx) { + Poll::Pending => panic!("rwlock busy"), + Poll::Ready(mut guard) => (), + }; + assert!(fut2.as_mut().is_terminated()); + } + } } }; } From 137327e83386cfd7d53cdc108ddfffb30304f2f4 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 19:51:35 -0400 Subject: [PATCH 10/27] Add poll after completion tests --- tests/rwlock.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index 67c0e70..f1a0138 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -305,6 +305,74 @@ macro_rules! gen_rwlock_tests { assert!(fut2.as_mut().is_terminated()); } } + + #[test] + #[should_panic] + fn poll_read_after_completion_should_panic() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + { + let fut = lock.read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => panic!("rwlock busy"), + Poll::Ready(mut guard) => (), + }; + assert!(fut.as_mut().is_terminated()); + + let _ = fut.as_mut().poll(cx); + } + } + } + + #[test] + #[should_panic] + fn poll_write_after_completion_should_panic() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + { + let fut = lock.write(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => panic!("rwlock busy"), + Poll::Ready(mut guard) => (), + }; + assert!(fut.as_mut().is_terminated()); + + let _ = fut.as_mut().poll(cx); + } + } + } + #[test] + #[should_panic] + fn poll_upgradable_read_after_completion_should_panic() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + { + let fut = lock.upgradable_read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => panic!("rwlock busy"), + Poll::Ready(mut guard) => (), + }; + assert!(fut.as_mut().is_terminated()); + + let _ = fut.as_mut().poll(cx); + } + } + } } }; } From 0a63d0681f4a4c099cd2ee0604454e7e988123c6 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 21:37:53 -0400 Subject: [PATCH 11/27] Add improved filtering method --- src/sync/rwlock.rs | 117 ++++++++++++++++++++--------- tests/rwlock.rs | 178 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 260 insertions(+), 35 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 41a1ccf..fbcef3e 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -80,6 +80,82 @@ impl Entry { } } +struct FilterWaiters { + nb_waiting_reads: usize, + nb_waiting_upgrade_reads: usize, + nb_reads: usize, + has_upgrade_read: bool, + remove_notified: bool, +} + +impl FilterWaiters { + fn filter(&mut self, entry: &mut ListNode) -> ControlFlow { + match entry.kind { + EntryKind::Read => { + entry.notify(); + self.nb_reads += 1; + self.nb_waiting_reads -= 1; + if self.nb_waiting_reads == 0 + && (self.has_upgrade_read + || self.nb_waiting_upgrade_reads == 0) + { + if self.remove_notified { + ControlFlow::RemoveAndStop + } else { + ControlFlow::Stop + } + } else { + if self.remove_notified { + ControlFlow::RemoveAndContinue + } else { + ControlFlow::Continue + } + } + } + EntryKind::UpgradeRead => { + if self.has_upgrade_read { + ControlFlow::Continue + } else { + entry.notify(); + self.has_upgrade_read = true; + self.nb_reads += 1; + self.nb_waiting_upgrade_reads -= 1; + if self.nb_waiting_reads == 0 { + if self.remove_notified { + ControlFlow::RemoveAndStop + } else { + ControlFlow::Stop + } + } else { + if self.remove_notified { + ControlFlow::RemoveAndContinue + } else { + ControlFlow::Continue + } + } + } + } + EntryKind::Write => { + if self.nb_reads == 0 { + entry.notify(); + if self.remove_notified { + ControlFlow::RemoveAndStop + } else { + ControlFlow::Stop + } + } else if self.nb_waiting_reads != 0 + || (!self.has_upgrade_read + && self.nb_waiting_upgrade_reads != 0) + { + ControlFlow::Continue + } else { + ControlFlow::Stop + } + } + } + } +} + /// Internal state of the `Mutex` struct MutexState { is_fair: bool, @@ -177,40 +253,15 @@ impl MutexState { } fn wakeup_any_waiters(&mut self) { - let mut nb_reads = 0; - if self.is_fair { - self.waiters.reverse_apply_while(|entry| match entry.kind { - EntryKind::Read | EntryKind::UpgradeRead => { - entry.notify(); - nb_reads += 1; - ControlFlow::Continue - } - EntryKind::Write => { - if nb_reads == 0 { - entry.notify(); - ControlFlow::Stop - } else { - ControlFlow::Continue - } - } - }); - } else { - self.waiters.reverse_apply_while(|entry| match entry.kind { - EntryKind::Read | EntryKind::UpgradeRead => { - entry.notify(); - nb_reads += 1; - ControlFlow::RemoveAndContinue - } - EntryKind::Write => { - if nb_reads == 0 { - entry.notify(); - ControlFlow::RemoveAndStop - } else { - ControlFlow::Continue - } - } - }); + let mut filter = FilterWaiters { + nb_waiting_reads: self.nb_waiting_reads, + nb_waiting_upgrade_reads: self.nb_waiting_upgrade_reads, + nb_reads: self.nb_reads, + has_upgrade_read: self.has_upgrade_read, + remove_notified: !self.is_fair, }; + self.waiters + .reverse_apply_while(|entry| filter.filter(entry)); } /// Attempt To gain shared read access. diff --git a/tests/rwlock.rs b/tests/rwlock.rs index f1a0138..15cd476 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -195,8 +195,8 @@ macro_rules! gen_rwlock_tests { } { - assert!(lock.try_upgradable_read().is_none()); - let fut = lock.upgradable_read(); + assert!(lock.try_read().is_none()); + let fut = lock.read(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => (), @@ -230,6 +230,41 @@ macro_rules! gen_rwlock_tests { } } + #[test] + fn contended_upgrade_read() { + for is_fair in &[true, false] { + let waker = &panic_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + let guard = lock.try_upgradable_read().unwrap(); + { + assert!(lock.try_upgradable_read().is_none()); + let fut = lock.upgradable_read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut.as_mut().is_terminated()); + } + drop(guard); + + let guard = lock.try_write().unwrap(); + { + assert!(lock.try_upgradable_read().is_none()); + let fut = lock.upgradable_read(); + pin_mut!(fut); + match fut.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + assert!(!fut.as_mut().is_terminated()); + } + } + } + #[test] fn dropping_reader_wakes_up_writer() { for is_fair in &[true, false] { @@ -351,6 +386,7 @@ macro_rules! gen_rwlock_tests { } } } + #[test] #[should_panic] fn poll_upgradable_read_after_completion_should_panic() { @@ -373,6 +409,144 @@ macro_rules! gen_rwlock_tests { } } } + + #[test] + fn dropping_writer_follow_queue_order_for_wakeup() { + for is_fair in &[true, false] { + let lock = $rwlock_type::new(5, *is_fair); + assert_eq!(false, lock.is_exclusive()); + + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + + let writer = lock.try_write().unwrap(); + + let read_fut1 = lock.read(); + pin_mut!(read_fut1); + match read_fut1.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + + let read_fut2 = lock.read(); + pin_mut!(read_fut2); + match read_fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + + let upgrade_read_fut1 = lock.upgradable_read(); + pin_mut!(upgrade_read_fut1); + match upgrade_read_fut1.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + + let upgrade_read_fut2 = lock.upgradable_read(); + pin_mut!(upgrade_read_fut2); + match upgrade_read_fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + + let write_fut1 = lock.write(); + pin_mut!(write_fut1); + match write_fut1.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + + let write_fut2 = lock.write(); + pin_mut!(write_fut2); + match write_fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + + let read_fut3 = lock.read(); + pin_mut!(read_fut3); + match read_fut3.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("rwlock ready"), + }; + + assert_eq!(count, 0); + + drop(writer); + + // Wakeup the three readers and 1 upgradable_read. + assert_eq!(count, 4); + + let guard1 = match read_fut1.as_mut().poll(cx) { + Poll::Pending => panic!("busy rwlock"), + Poll::Ready(mut guard) => guard, + }; + let guard2 = match read_fut2.as_mut().poll(cx) { + Poll::Pending => panic!("busy rwlock"), + Poll::Ready(mut guard) => guard, + }; + let guard3 = match upgrade_read_fut1.as_mut().poll(cx) { + Poll::Pending => panic!("busy rwlock"), + Poll::Ready(mut guard) => guard, + }; + match upgrade_read_fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("busy rwlock"), + }; + match write_fut1.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("busy rwlock"), + }; + match write_fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("busy rwlock"), + }; + + assert_eq!(count, 3); + + drop(guard1); + assert_eq!(count, 3); + + drop(guard3); + assert_eq!(count, 4); + + let guard4 = match upgrade_read_fut2.as_mut().poll(cx) { + Poll::Pending => panic!("busy rwlock"), + Poll::Ready(mut guard) => guard, + }; + match write_fut1.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("busy rwlock"), + }; + match write_fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("busy rwlock"), + }; + + drop(guard2); + assert_eq!(count, 4); + drop(guard4); + assert_eq!(count, 5); + + let guard5 = match write_fut1.as_mut().poll(cx) { + Poll::Pending => panic!("busy rwlock"), + Poll::Ready(mut guard) => guard, + }; + match write_fut2.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(mut guard) => panic!("busy rwlock"), + }; + + drop(guard5); + assert_eq!(count, 6); + + match write_fut2.as_mut().poll(cx) { + Poll::Pending => panic!("busy rwlock"), + Poll::Ready(mut guard) => (), + }; + assert_eq!(count, 6); + } + } } }; } From 787d4aeb4ea59391d5d25fc4c7cda762b97812b0 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 21:57:26 -0400 Subject: [PATCH 12/27] Fix wakeup on future dropped before completion --- src/sync/rwlock.rs | 37 +++++++++++-------------------------- tests/rwlock.rs | 26 ++++++++++++++++---------- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index fbcef3e..2ff65a7 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -114,7 +114,11 @@ impl FilterWaiters { } EntryKind::UpgradeRead => { if self.has_upgrade_read { - ControlFlow::Continue + if self.nb_waiting_reads > 0 { + ControlFlow::Continue + } else { + ControlFlow::Stop + } } else { entry.notify(); self.has_upgrade_read = true; @@ -143,9 +147,9 @@ impl FilterWaiters { } else { ControlFlow::Stop } - } else if self.nb_waiting_reads != 0 + } else if self.nb_waiting_reads > 0 || (!self.has_upgrade_read - && self.nb_waiting_upgrade_reads != 0) + && self.nb_waiting_upgrade_reads > 0) { ControlFlow::Continue } else { @@ -190,26 +194,7 @@ impl MutexState { fn unlock_read(&mut self) { self.nb_reads -= 1; if self.nb_reads == 0 { - // Wakeup the next write in line. - if self.is_fair { - self.waiters.reverse_apply_while(|entry| { - if let EntryKind::Write = entry.kind { - entry.notify(); - ControlFlow::Stop - } else { - ControlFlow::Continue - } - }); - } else { - self.waiters.reverse_apply_while(|entry| { - if let EntryKind::Write = entry.kind { - entry.notify(); - ControlFlow::RemoveAndStop - } else { - ControlFlow::Continue - } - }); - }; + self.wakeup_any_waiters(); } } @@ -707,7 +692,7 @@ impl MutexState { wait_node.state = PollState::Done; // Since the task was notified but did not lock the Mutex, // another task gets the chance to run. - self.unlock_read(); + self.wakeup_any_waiters(); } PollState::Waiting => { // Remove the Entry from the linked list @@ -744,7 +729,7 @@ impl MutexState { wait_node.state = PollState::Done; // Since the task was notified but did not lock the Mutex, // another task gets the chance to run. - self.unlock_write(); + self.wakeup_any_waiters(); } PollState::Waiting => { // Remove the Entry from the linked list @@ -781,7 +766,7 @@ impl MutexState { wait_node.state = PollState::Done; // Since the task was notified but did not lock the Mutex, // another task gets the chance to run. - self.unlock_upgrade_read(); + self.wakeup_any_waiters(); } PollState::Waiting => { // Remove the Entry from the linked list diff --git a/tests/rwlock.rs b/tests/rwlock.rs index 15cd476..ae560ee 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -489,6 +489,10 @@ macro_rules! gen_rwlock_tests { Poll::Pending => panic!("busy rwlock"), Poll::Ready(mut guard) => guard, }; + let guard4 = match read_fut3.as_mut().poll(cx) { + Poll::Pending => panic!("busy rwlock"), + Poll::Ready(mut guard) => guard, + }; match upgrade_read_fut2.as_mut().poll(cx) { Poll::Pending => (), Poll::Ready(mut guard) => panic!("busy rwlock"), @@ -502,15 +506,15 @@ macro_rules! gen_rwlock_tests { Poll::Ready(mut guard) => panic!("busy rwlock"), }; - assert_eq!(count, 3); + assert_eq!(count, 4); drop(guard1); - assert_eq!(count, 3); + assert_eq!(count, 4); drop(guard3); - assert_eq!(count, 4); + assert_eq!(count, 5); - let guard4 = match upgrade_read_fut2.as_mut().poll(cx) { + let guard5 = match upgrade_read_fut2.as_mut().poll(cx) { Poll::Pending => panic!("busy rwlock"), Poll::Ready(mut guard) => guard, }; @@ -524,11 +528,13 @@ macro_rules! gen_rwlock_tests { }; drop(guard2); - assert_eq!(count, 4); - drop(guard4); assert_eq!(count, 5); + drop(guard5); + assert_eq!(count, 5); + drop(guard4); + assert_eq!(count, 6); - let guard5 = match write_fut1.as_mut().poll(cx) { + let guard6 = match write_fut1.as_mut().poll(cx) { Poll::Pending => panic!("busy rwlock"), Poll::Ready(mut guard) => guard, }; @@ -537,14 +543,14 @@ macro_rules! gen_rwlock_tests { Poll::Ready(mut guard) => panic!("busy rwlock"), }; - drop(guard5); - assert_eq!(count, 6); + drop(guard6); + assert_eq!(count, 7); match write_fut2.as_mut().poll(cx) { Poll::Pending => panic!("busy rwlock"), Poll::Ready(mut guard) => (), }; - assert_eq!(count, 6); + assert_eq!(count, 7); } } } From b91e50c2e67a04120f7b2c7dc8351d6d51a98cd9 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 22:22:39 -0400 Subject: [PATCH 13/27] Improve test debugging by using collapse_debuginfo --- src/sync/rwlock.rs | 3 +- tests/rwlock.rs | 303 ++++++++++++++++++++++++--------------------- 2 files changed, 166 insertions(+), 140 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 2ff65a7..c39315d 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -80,6 +80,7 @@ impl Entry { } } +#[derive(Debug)] struct FilterWaiters { nb_waiting_reads: usize, nb_waiting_upgrade_reads: usize, @@ -219,7 +220,7 @@ impl MutexState { self.has_upgrade_read = false; self.nb_reads -= 1; - if self.nb_reads == 0 { + if self.nb_reads == 0 || self.nb_waiting_upgrade_reads > 0 { self.wakeup_any_waiters(); } } diff --git a/tests/rwlock.rs b/tests/rwlock.rs index ae560ee..dd610d1 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -6,7 +6,28 @@ use futures_intrusive::sync::LocalRwLock; use futures_test::task::{new_count_waker, panic_waker}; use pin_utils::pin_mut; -// Allows backtrace to work properly. +// Allows backtrace to work properly inside macro expension. +#[collapse_debuginfo] +macro_rules! assert_eq_ { + ($left:expr, $right:expr $(,)?) => { + assert_eq!($left, $right); + }; +} + +#[collapse_debuginfo] +macro_rules! assert_ { + ($val:expr) => { + assert!($val) + }; +} + +#[collapse_debuginfo] +macro_rules! panic_ { + ($val:expr) => { + panic!($val) + }; +} + #[collapse_debuginfo] macro_rules! gen_rwlock_tests { ($mod_name:ident, $rwlock_type:ident) => { @@ -19,7 +40,7 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let mut guards = Vec::with_capacity(3); @@ -28,28 +49,31 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect lock to get locked") + panic_!("Expect lock to get locked") } Poll::Ready(mut guard) => { - assert_eq!(false, lock.is_exclusive()); - assert_eq!(5, *guard); + assert_eq_!(false, lock.is_exclusive()); + assert_eq_!(5, *guard); guards.push(guard); - assert_eq!(guards.len(), lock.nb_readers()); + assert_eq_!( + guards.len(), + lock.nb_readers() + ); } }; - assert!(fut.as_mut().is_terminated()); + assert_!(fut.as_mut().is_terminated()); } - assert_eq!(3, lock.nb_readers()); + assert_eq_!(3, lock.nb_readers()); drop(guards.pop().unwrap()); - assert_eq!(2, lock.nb_readers()); + assert_eq_!(2, lock.nb_readers()); drop(guards.pop().unwrap()); - assert_eq!(1, lock.nb_readers()); + assert_eq_!(1, lock.nb_readers()); drop(guards.pop().unwrap()); - assert_eq!(0, lock.nb_readers()); + assert_eq_!(0, lock.nb_readers()); } { @@ -57,17 +81,17 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect lock to get locked") + panic_!("Expect lock to get locked") } Poll::Ready(guard) => { - assert_eq!(false, lock.is_exclusive()); - assert_eq!(1, lock.nb_readers()); - assert_eq!(5, *guard); + assert_eq_!(false, lock.is_exclusive()); + assert_eq_!(1, lock.nb_readers()); + assert_eq_!(5, *guard); } }; } - assert_eq!(0, lock.nb_readers()); + assert_eq_!(0, lock.nb_readers()); } } @@ -77,41 +101,41 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let fut = lock.write(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect lock to get locked") + panic_!("Expect lock to get locked") } Poll::Ready(mut guard) => { - assert_eq!(true, lock.is_exclusive()); - assert_eq!(5, *guard); + assert_eq_!(true, lock.is_exclusive()); + assert_eq_!(5, *guard); *guard = 12; - assert_eq!(12, *guard); + assert_eq_!(12, *guard); } }; - assert!(fut.as_mut().is_terminated()); + assert_!(fut.as_mut().is_terminated()); } - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let fut = lock.write(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect lock to get locked") + panic_!("Expect lock to get locked") } Poll::Ready(guard) => { - assert_eq!(true, lock.is_exclusive()); - assert_eq!(12, *guard); + assert_eq_!(true, lock.is_exclusive()); + assert_eq_!(12, *guard); } }; } - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); } } @@ -121,55 +145,55 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let fut = lock.upgradable_read(); pin_mut!(fut); let guard = match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect lock to get locked") + panic_!("Expect lock to get locked") } - Poll::Ready(mut guard) => { - assert_eq!(false, lock.is_exclusive()); - assert_eq!(5, *guard); + Poll::Ready(guard) => { + assert_eq_!(false, lock.is_exclusive()); + assert_eq_!(5, *guard); guard } }; - assert!(fut.as_mut().is_terminated()); + assert_!(fut.as_mut().is_terminated()); let fut = guard.upgrade(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect lock to get locked") + panic_!("Expect lock to get locked") } Poll::Ready(mut guard) => { - assert_eq!(true, lock.is_exclusive()); - assert_eq!(5, *guard); + assert_eq_!(true, lock.is_exclusive()); + assert_eq_!(5, *guard); *guard = 12; - assert_eq!(12, *guard); + assert_eq_!(12, *guard); } }; } - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let fut = lock.upgradable_read(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => { - panic!("Expect lock to get locked") + panic_!("Expect lock to get locked") } Poll::Ready(guard) => { - assert_eq!(false, lock.is_exclusive()); - assert_eq!(12, *guard); + assert_eq_!(false, lock.is_exclusive()); + assert_eq_!(12, *guard); } }; } - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); } } @@ -179,30 +203,30 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); let guard = lock.try_write().unwrap(); { - assert!(lock.try_read().is_none()); + assert_!(lock.try_read().is_none()); let fut = lock.read(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut.as_mut().is_terminated()); + assert_!(!fut.as_mut().is_terminated()); } { - assert!(lock.try_read().is_none()); + assert_!(lock.try_read().is_none()); let fut = lock.read(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut.as_mut().is_terminated()); + assert_!(!fut.as_mut().is_terminated()); } } } @@ -213,19 +237,19 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); let guard = lock.try_read().unwrap(); { - assert!(lock.try_write().is_none()); + assert_!(lock.try_write().is_none()); let fut = lock.write(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut.as_mut().is_terminated()); + assert_!(!fut.as_mut().is_terminated()); } } } @@ -236,31 +260,31 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); let guard = lock.try_upgradable_read().unwrap(); { - assert!(lock.try_upgradable_read().is_none()); + assert_!(lock.try_upgradable_read().is_none()); let fut = lock.upgradable_read(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut.as_mut().is_terminated()); + assert_!(!fut.as_mut().is_terminated()); } drop(guard); let guard = lock.try_write().unwrap(); { - assert!(lock.try_upgradable_read().is_none()); + assert_!(lock.try_upgradable_read().is_none()); let fut = lock.upgradable_read(); pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut.as_mut().is_terminated()); + assert_!(!fut.as_mut().is_terminated()); } } } @@ -271,7 +295,7 @@ macro_rules! gen_rwlock_tests { let (waker, count) = new_count_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); let guard = lock.try_read().unwrap(); @@ -279,20 +303,20 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut.as_mut().is_terminated()); + assert_!(!fut.as_mut().is_terminated()); - assert_eq!(count, 0); + assert_eq_!(count, 0); drop(guard); - assert_eq!(count, 1); + assert_eq_!(count, 1); match fut.as_mut().poll(cx) { - Poll::Pending => panic!("rwlock busy"), - Poll::Ready(mut guard) => (), + Poll::Pending => panic_!("rwlock busy"), + Poll::Ready(guard) => (), }; - assert!(fut.as_mut().is_terminated()); + assert_!(fut.as_mut().is_terminated()); } } @@ -302,7 +326,7 @@ macro_rules! gen_rwlock_tests { let (waker, count) = new_count_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); let guard = lock.try_write().unwrap(); @@ -310,34 +334,34 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut1); match fut1.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut1.as_mut().is_terminated()); + assert_!(!fut1.as_mut().is_terminated()); let fut2 = lock.upgradable_read(); pin_mut!(fut2); match fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert!(!fut2.as_mut().is_terminated()); + assert_!(!fut2.as_mut().is_terminated()); - assert_eq!(count, 0); + assert_eq_!(count, 0); drop(guard); - assert_eq!(count, 2); + assert_eq_!(count, 2); match fut1.as_mut().poll(cx) { - Poll::Pending => panic!("rwlock busy"), - Poll::Ready(mut guard) => (), + Poll::Pending => panic_!("rwlock busy"), + Poll::Ready(guard) => (), }; - assert!(fut1.as_mut().is_terminated()); + assert_!(fut1.as_mut().is_terminated()); match fut2.as_mut().poll(cx) { - Poll::Pending => panic!("rwlock busy"), - Poll::Ready(mut guard) => (), + Poll::Pending => panic_!("rwlock busy"), + Poll::Ready(guard) => (), }; - assert!(fut2.as_mut().is_terminated()); + assert_!(fut2.as_mut().is_terminated()); } } @@ -348,16 +372,16 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let fut = lock.read(); pin_mut!(fut); match fut.as_mut().poll(cx) { - Poll::Pending => panic!("rwlock busy"), - Poll::Ready(mut guard) => (), + Poll::Pending => panic_!("rwlock busy"), + Poll::Ready(guard) => (), }; - assert!(fut.as_mut().is_terminated()); + assert_!(fut.as_mut().is_terminated()); let _ = fut.as_mut().poll(cx); } @@ -371,16 +395,16 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let fut = lock.write(); pin_mut!(fut); match fut.as_mut().poll(cx) { - Poll::Pending => panic!("rwlock busy"), - Poll::Ready(mut guard) => (), + Poll::Pending => panic_!("rwlock busy"), + Poll::Ready(guard) => (), }; - assert!(fut.as_mut().is_terminated()); + assert_!(fut.as_mut().is_terminated()); let _ = fut.as_mut().poll(cx); } @@ -394,16 +418,16 @@ macro_rules! gen_rwlock_tests { let waker = &panic_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); { let fut = lock.upgradable_read(); pin_mut!(fut); match fut.as_mut().poll(cx) { - Poll::Pending => panic!("rwlock busy"), - Poll::Ready(mut guard) => (), + Poll::Pending => panic_!("rwlock busy"), + Poll::Ready(guard) => (), }; - assert!(fut.as_mut().is_terminated()); + assert_!(fut.as_mut().is_terminated()); let _ = fut.as_mut().poll(cx); } @@ -411,10 +435,10 @@ macro_rules! gen_rwlock_tests { } #[test] - fn dropping_writer_follow_queue_order_for_wakeup() { + fn dropping_guard_follows_queue_order_for_wakeup() { for is_fair in &[true, false] { let lock = $rwlock_type::new(5, *is_fair); - assert_eq!(false, lock.is_exclusive()); + assert_eq_!(false, lock.is_exclusive()); let (waker, count) = new_count_waker(); let cx = &mut Context::from_waker(&waker); @@ -425,132 +449,133 @@ macro_rules! gen_rwlock_tests { pin_mut!(read_fut1); match read_fut1.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; let read_fut2 = lock.read(); pin_mut!(read_fut2); match read_fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), + }; + + let write_fut1 = lock.write(); + pin_mut!(write_fut1); + match write_fut1.as_mut().poll(cx) { + Poll::Pending => (), + Poll::Ready(guard) => panic_!("rwlock ready"), }; let upgrade_read_fut1 = lock.upgradable_read(); pin_mut!(upgrade_read_fut1); match upgrade_read_fut1.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; let upgrade_read_fut2 = lock.upgradable_read(); pin_mut!(upgrade_read_fut2); match upgrade_read_fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), - }; - - let write_fut1 = lock.write(); - pin_mut!(write_fut1); - match write_fut1.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; let write_fut2 = lock.write(); pin_mut!(write_fut2); match write_fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; let read_fut3 = lock.read(); pin_mut!(read_fut3); match read_fut3.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("rwlock ready"), + Poll::Ready(guard) => panic_!("rwlock ready"), }; - assert_eq!(count, 0); + assert_eq_!(count, 0); drop(writer); // Wakeup the three readers and 1 upgradable_read. - assert_eq!(count, 4); + assert_eq_!(count, 4); let guard1 = match read_fut1.as_mut().poll(cx) { - Poll::Pending => panic!("busy rwlock"), - Poll::Ready(mut guard) => guard, + Poll::Pending => panic_!("busy rwlock"), + Poll::Ready(guard) => guard, }; let guard2 = match read_fut2.as_mut().poll(cx) { - Poll::Pending => panic!("busy rwlock"), - Poll::Ready(mut guard) => guard, + Poll::Pending => panic_!("busy rwlock"), + Poll::Ready(guard) => guard, }; let guard3 = match upgrade_read_fut1.as_mut().poll(cx) { - Poll::Pending => panic!("busy rwlock"), - Poll::Ready(mut guard) => guard, + Poll::Pending => panic_!("busy rwlock"), + Poll::Ready(guard) => guard, }; let guard4 = match read_fut3.as_mut().poll(cx) { - Poll::Pending => panic!("busy rwlock"), - Poll::Ready(mut guard) => guard, + Poll::Pending => panic_!("busy rwlock"), + Poll::Ready(guard) => guard, }; match upgrade_read_fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("busy rwlock"), + Poll::Ready(guard) => panic_!("busy rwlock"), }; match write_fut1.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("busy rwlock"), + Poll::Ready(guard) => panic_!("busy rwlock"), }; match write_fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("busy rwlock"), + Poll::Ready(guard) => panic_!("busy rwlock"), }; - assert_eq!(count, 4); + assert_eq_!(count, 4); drop(guard1); - assert_eq!(count, 4); + assert_eq_!(count, 4); + // Wakeup the other upgradable_read drop(guard3); - assert_eq!(count, 5); + assert_eq_!(count, 5); let guard5 = match upgrade_read_fut2.as_mut().poll(cx) { - Poll::Pending => panic!("busy rwlock"), - Poll::Ready(mut guard) => guard, + Poll::Pending => panic_!("busy rwlock"), + Poll::Ready(guard) => guard, }; match write_fut1.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("busy rwlock"), + Poll::Ready(guard) => panic_!("busy rwlock"), }; match write_fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("busy rwlock"), + Poll::Ready(guard) => panic_!("busy rwlock"), }; drop(guard2); - assert_eq!(count, 5); + assert_eq_!(count, 5); drop(guard5); - assert_eq!(count, 5); + assert_eq_!(count, 5); drop(guard4); - assert_eq!(count, 6); + assert_eq_!(count, 6); let guard6 = match write_fut1.as_mut().poll(cx) { - Poll::Pending => panic!("busy rwlock"), - Poll::Ready(mut guard) => guard, + Poll::Pending => panic_!("busy rwlock"), + Poll::Ready(guard) => guard, }; match write_fut2.as_mut().poll(cx) { Poll::Pending => (), - Poll::Ready(mut guard) => panic!("busy rwlock"), + Poll::Ready(guard) => panic_!("busy rwlock"), }; drop(guard6); - assert_eq!(count, 7); + assert_eq_!(count, 7); match write_fut2.as_mut().poll(cx) { - Poll::Pending => panic!("busy rwlock"), - Poll::Ready(mut guard) => (), + Poll::Pending => panic_!("busy rwlock"), + Poll::Ready(guard) => (), }; - assert_eq!(count, 7); + assert_eq_!(count, 7); } } } @@ -594,7 +619,7 @@ mod if_std { let res = fut.poll_unpin(cx); let guard = match res { Poll::Ready(v) => v, - Poll::Pending => panic!("Expected to be ready"), + Poll::Pending => panic_!("Expected to be ready"), }; is_send(&guard); is_send_value(guard); From 066b1002ea7e0c27de97b27798a94e299be388ef Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 22:33:15 -0400 Subject: [PATCH 14/27] Cleanup tests --- tests/rwlock.rs | 105 ++++++++++-------------------------------------- 1 file changed, 21 insertions(+), 84 deletions(-) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index dd610d1..c048f88 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -211,10 +211,7 @@ macro_rules! gen_rwlock_tests { assert_!(lock.try_read().is_none()); let fut = lock.read(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut.as_mut().poll(cx).is_pending()); assert_!(!fut.as_mut().is_terminated()); } @@ -222,10 +219,7 @@ macro_rules! gen_rwlock_tests { assert_!(lock.try_read().is_none()); let fut = lock.read(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut.as_mut().poll(cx).is_pending()); assert_!(!fut.as_mut().is_terminated()); } } @@ -245,10 +239,7 @@ macro_rules! gen_rwlock_tests { assert_!(lock.try_write().is_none()); let fut = lock.write(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut.as_mut().poll(cx).is_pending()); assert_!(!fut.as_mut().is_terminated()); } } @@ -267,10 +258,7 @@ macro_rules! gen_rwlock_tests { assert_!(lock.try_upgradable_read().is_none()); let fut = lock.upgradable_read(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut.as_mut().poll(cx).is_pending()); assert_!(!fut.as_mut().is_terminated()); } drop(guard); @@ -280,10 +268,7 @@ macro_rules! gen_rwlock_tests { assert_!(lock.try_upgradable_read().is_none()); let fut = lock.upgradable_read(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut.as_mut().poll(cx).is_pending()); assert_!(!fut.as_mut().is_terminated()); } } @@ -301,10 +286,7 @@ macro_rules! gen_rwlock_tests { let mut fut = lock.write(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut.as_mut().poll(cx).is_pending()); assert_!(!fut.as_mut().is_terminated()); assert_eq_!(count, 0); @@ -332,18 +314,12 @@ macro_rules! gen_rwlock_tests { let fut1 = lock.read(); pin_mut!(fut1); - match fut1.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut1.as_mut().poll(cx).is_pending()); assert_!(!fut1.as_mut().is_terminated()); let fut2 = lock.upgradable_read(); pin_mut!(fut2); - match fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(fut2.as_mut().poll(cx).is_pending()); assert_!(!fut2.as_mut().is_terminated()); assert_eq_!(count, 0); @@ -447,52 +423,31 @@ macro_rules! gen_rwlock_tests { let read_fut1 = lock.read(); pin_mut!(read_fut1); - match read_fut1.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(read_fut1.as_mut().poll(cx).is_pending()); let read_fut2 = lock.read(); pin_mut!(read_fut2); - match read_fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(read_fut2.as_mut().poll(cx).is_pending()); let write_fut1 = lock.write(); pin_mut!(write_fut1); - match write_fut1.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(write_fut1.as_mut().poll(cx).is_pending()); let upgrade_read_fut1 = lock.upgradable_read(); pin_mut!(upgrade_read_fut1); - match upgrade_read_fut1.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(upgrade_read_fut1.as_mut().poll(cx).is_pending()); let upgrade_read_fut2 = lock.upgradable_read(); pin_mut!(upgrade_read_fut2); - match upgrade_read_fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(upgrade_read_fut2.as_mut().poll(cx).is_pending()); let write_fut2 = lock.write(); pin_mut!(write_fut2); - match write_fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(write_fut2.as_mut().poll(cx).is_pending()); let read_fut3 = lock.read(); pin_mut!(read_fut3); - match read_fut3.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("rwlock ready"), - }; + assert_!(read_fut3.as_mut().poll(cx).is_pending()); assert_eq_!(count, 0); @@ -517,18 +472,9 @@ macro_rules! gen_rwlock_tests { Poll::Pending => panic_!("busy rwlock"), Poll::Ready(guard) => guard, }; - match upgrade_read_fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("busy rwlock"), - }; - match write_fut1.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("busy rwlock"), - }; - match write_fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("busy rwlock"), - }; + assert_!(upgrade_read_fut2.as_mut().poll(cx).is_pending()); + assert_!(write_fut1.as_mut().poll(cx).is_pending()); + assert_!(write_fut2.as_mut().poll(cx).is_pending()); assert_eq_!(count, 4); @@ -543,14 +489,8 @@ macro_rules! gen_rwlock_tests { Poll::Pending => panic_!("busy rwlock"), Poll::Ready(guard) => guard, }; - match write_fut1.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("busy rwlock"), - }; - match write_fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("busy rwlock"), - }; + assert_!(write_fut1.as_mut().poll(cx).is_pending()); + assert_!(write_fut2.as_mut().poll(cx).is_pending()); drop(guard2); assert_eq_!(count, 5); @@ -563,10 +503,7 @@ macro_rules! gen_rwlock_tests { Poll::Pending => panic_!("busy rwlock"), Poll::Ready(guard) => guard, }; - match write_fut2.as_mut().poll(cx) { - Poll::Pending => (), - Poll::Ready(guard) => panic_!("busy rwlock"), - }; + assert_!(write_fut2.as_mut().poll(cx).is_pending()); drop(guard6); assert_eq_!(count, 7); From f7f0288cba89fa66ad76d84c89d1e3064ec40ef8 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 22:51:46 -0400 Subject: [PATCH 15/27] Add future cancellation test --- tests/rwlock.rs | 63 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index c048f88..8dafb2d 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -296,7 +296,7 @@ macro_rules! gen_rwlock_tests { match fut.as_mut().poll(cx) { Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(guard) => (), + Poll::Ready(_) => (), }; assert_!(fut.as_mut().is_terminated()); } @@ -329,13 +329,13 @@ macro_rules! gen_rwlock_tests { match fut1.as_mut().poll(cx) { Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(guard) => (), + Poll::Ready(_) => (), }; assert_!(fut1.as_mut().is_terminated()); match fut2.as_mut().poll(cx) { Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(guard) => (), + Poll::Ready(_) => (), }; assert_!(fut2.as_mut().is_terminated()); } @@ -355,7 +355,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(guard) => (), + Poll::Ready(_) => (), }; assert_!(fut.as_mut().is_terminated()); @@ -378,7 +378,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(guard) => (), + Poll::Ready(_) => (), }; assert_!(fut.as_mut().is_terminated()); @@ -401,7 +401,7 @@ macro_rules! gen_rwlock_tests { pin_mut!(fut); match fut.as_mut().poll(cx) { Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(guard) => (), + Poll::Ready(_) => (), }; assert_!(fut.as_mut().is_terminated()); @@ -437,10 +437,6 @@ macro_rules! gen_rwlock_tests { pin_mut!(upgrade_read_fut1); assert_!(upgrade_read_fut1.as_mut().poll(cx).is_pending()); - let upgrade_read_fut2 = lock.upgradable_read(); - pin_mut!(upgrade_read_fut2); - assert_!(upgrade_read_fut2.as_mut().poll(cx).is_pending()); - let write_fut2 = lock.write(); pin_mut!(write_fut2); assert_!(write_fut2.as_mut().poll(cx).is_pending()); @@ -449,6 +445,10 @@ macro_rules! gen_rwlock_tests { pin_mut!(read_fut3); assert_!(read_fut3.as_mut().poll(cx).is_pending()); + let upgrade_read_fut2 = lock.upgradable_read(); + pin_mut!(upgrade_read_fut2); + assert_!(upgrade_read_fut2.as_mut().poll(cx).is_pending()); + assert_eq_!(count, 0); drop(writer); @@ -496,6 +496,7 @@ macro_rules! gen_rwlock_tests { assert_eq_!(count, 5); drop(guard5); assert_eq_!(count, 5); + // Now that we dropped all readers, wakeup one writer. drop(guard4); assert_eq_!(count, 6); @@ -505,17 +506,57 @@ macro_rules! gen_rwlock_tests { }; assert_!(write_fut2.as_mut().poll(cx).is_pending()); + // Wakeup the other writer. drop(guard6); assert_eq_!(count, 7); match write_fut2.as_mut().poll(cx) { Poll::Pending => panic_!("busy rwlock"), - Poll::Ready(guard) => (), + Poll::Ready(_) => (), }; assert_eq_!(count, 7); } } } + + #[test] + fn cancel_wait_for_lock() { + for is_fair in &[true, false] { + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + + let mut guard1 = lock.try_write().unwrap(); + + // The second and third lock attempt must fail + let mut fut1 = Box::pin(lock.write()); + let mut fut2 = Box::pin(lock.read()); + let mut fut3 = Box::pin(lock.upgradable_read()); + let mut fut4 = Box::pin(lock.write()); + + assert!(fut1.as_mut().poll(cx).is_pending()); + assert!(fut2.as_mut().poll(cx).is_pending()); + assert!(fut3.as_mut().poll(cx).is_pending()); + assert!(fut4.as_mut().poll(cx).is_pending()); + + // Before the lock gets available, cancel a bunch of futures. + drop(fut1); + drop(fut2); + drop(fut3); + + assert_eq!(count, 0); + + // The reader should have been notified. + drop(guard1); + assert_eq!(count, 1); + + // Unlock - mutex should be available again + match fut4.as_mut().poll(cx) { + Poll::Pending => panic!("Expect mutex to get locked"), + Poll::Ready(_) => (), + }; + } + } }; } From 8d5e6a483b3820f0cbd696a2fc30bb293e70f237 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 23:10:17 -0400 Subject: [PATCH 16/27] Add test for ignored notifications --- tests/rwlock.rs | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index 8dafb2d..d273586 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -557,6 +557,76 @@ macro_rules! gen_rwlock_tests { }; } } + + #[test] + fn unlock_next_when_notification_is_not_used() { + for is_fair in &[true, false] { + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, *is_fair); + + // Lock the mutex + let mut guard1 = lock.try_write().unwrap(); + + // The second and third lock attempt must fail + let mut fut2 = Box::pin(lock.write()); + let mut fut3 = Box::pin(lock.write()); + let mut fut4 = Box::pin(lock.upgradable_read()); + let mut fut5 = Box::pin(lock.upgradable_read()); + + assert_!(fut2.as_mut().poll(cx).is_pending()); + assert_!(fut3.as_mut().poll(cx).is_pending()); + assert_!(fut4.as_mut().poll(cx).is_pending()); + assert_!(fut5.as_mut().poll(cx).is_pending()); + + assert_eq_!(count, 0); + + // Unlock, notifying the next waiter in line. + drop(guard1); + assert_eq_!(count, 1); + + // Ignore the notification. + drop(fut2); + assert_eq_!(count, 2); + + // Unlock - mutex should be available again + let guard3 = match fut3.as_mut().poll(cx) { + Poll::Pending => panic!("lock busy"), + Poll::Ready(guard) => guard, + }; + + drop(guard3); + assert_eq_!(count, 3); + drop(fut4); + assert_eq_!(count, 4); + + match fut5.as_mut().poll(cx) { + Poll::Pending => panic!("lock busy"), + Poll::Ready(guard) => (), + }; + + // We also test cancelling read locks. + let guard = lock.try_write().unwrap(); + + let mut fut6 = Box::pin(lock.upgradable_read()); + let mut fut7 = Box::pin(lock.upgradable_read()); + + assert_!(fut6.as_mut().poll(cx).is_pending()); + assert_!(fut7.as_mut().poll(cx).is_pending()); + + assert_eq_!(count, 4); + drop(guard); + assert_eq_!(count, 5); + drop(fut6); + assert_eq_!(count, 6); + + match fut7.as_mut().poll(cx) { + Poll::Pending => panic!("lock busy"), + Poll::Ready(_) => (), + }; + + } + } }; } From a61f305772e45807abbd586d5d72672ffdb1737d Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 23:26:39 -0400 Subject: [PATCH 17/27] Add test for ignored future wakeups --- src/sync/rwlock.rs | 3 ++- tests/rwlock.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index c39315d..87f8c8a 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -18,7 +18,7 @@ use futures_core::{ use lock_api::{Mutex as LockApiMutex, RawMutex}; /// Tracks how the future had interacted with the mutex -#[derive(PartialEq)] +#[derive(Debug, PartialEq)] enum PollState { /// The task has never interacted with the mutex. New, @@ -49,6 +49,7 @@ enum EntryKind { /// Tracks the MutexLockFuture waiting state. /// Access to this struct is synchronized through the mutex in the Event. +#[derive(Debug)] struct Entry { /// The task handle of the waiting task task: Option, diff --git a/tests/rwlock.rs b/tests/rwlock.rs index d273586..894f591 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -627,6 +627,56 @@ macro_rules! gen_rwlock_tests { } } + + #[test] + fn new_waiters_on_unfair_lock_can_acquire_future_while_one_task_is_notified() { + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, false); + + let mut guard1 = lock.try_write().unwrap(); + + // The second and third lock attempt must fail + let mut fut2 = Box::pin(lock.write()); + let mut fut3 = Box::pin(lock.upgradable_read()); + let mut fut4 = Box::pin(lock.write()); + + assert_!(fut2.as_mut().poll(cx).is_pending()); + assert_!(fut3.as_mut().poll(cx).is_pending()); + + // Notify fut2. + drop(guard1); + assert_eq_!(count, 1); + + // Steal the lock. + let guard4 = match fut4.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(guard) => guard, + }; + // The futures must requeue. + assert_!(fut2.as_mut().poll(cx).is_pending()); + assert_!(fut3.as_mut().poll(cx).is_pending()); + + // When we drop fut3, the mutex should signal that it's available for fut2, + // which needs to have re-registered + drop(guard4); + assert_eq_!(count, 2); + + let guard2 = match fut2.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(guard) => guard, + }; + + assert_!(fut3.as_mut().poll(cx).is_pending()); + + drop(guard2); + assert_eq_!(count, 3); + + match fut3.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(_) => (), + }; + } }; } From eacf49740b3bb667177d5c2ede7b5eb519ed2bea Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Sun, 24 Mar 2024 23:49:00 -0400 Subject: [PATCH 18/27] Add lock stealing tests for fairness --- tests/rwlock.rs | 87 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 8 deletions(-) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index 894f591..e3fbf7b 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -639,43 +639,114 @@ macro_rules! gen_rwlock_tests { // The second and third lock attempt must fail let mut fut2 = Box::pin(lock.write()); let mut fut3 = Box::pin(lock.upgradable_read()); - let mut fut4 = Box::pin(lock.write()); + let mut fut4 = Box::pin(lock.read()); + let mut fut5 = Box::pin(lock.write()); assert_!(fut2.as_mut().poll(cx).is_pending()); assert_!(fut3.as_mut().poll(cx).is_pending()); + assert_!(fut4.as_mut().poll(cx).is_pending()); // Notify fut2. drop(guard1); assert_eq_!(count, 1); // Steal the lock. - let guard4 = match fut4.as_mut().poll(cx) { + let guard5 = match fut5.as_mut().poll(cx) { Poll::Pending => panic_!("Expect mutex to get locked"), Poll::Ready(guard) => guard, }; - // The futures must requeue. + // This future must requeue. assert_!(fut2.as_mut().poll(cx).is_pending()); + + // ...but the other one preserve their old position in the queue. assert_!(fut3.as_mut().poll(cx).is_pending()); + assert_!(fut4.as_mut().poll(cx).is_pending()); - // When we drop fut3, the mutex should signal that it's available for fut2, - // which needs to have re-registered - drop(guard4); - assert_eq_!(count, 2); + // When we drop fut3, the mutex should signal that it's available for + // fut2 and fut3 since they have priority in the queue. + assert_eq_!(count, 1); + drop(guard5); + assert_eq_!(count, 3); + // We streal the lock again. let guard2 = match fut2.as_mut().poll(cx) { Poll::Pending => panic_!("Expect mutex to get locked"), Poll::Ready(guard) => guard, }; + // The two notified futures must register again. assert_!(fut3.as_mut().poll(cx).is_pending()); + assert_!(fut4.as_mut().poll(cx).is_pending()); + // Now we notify the two futures. drop(guard2); - assert_eq_!(count, 3); + assert_eq_!(count, 5); match fut3.as_mut().poll(cx) { Poll::Pending => panic_!("Expect mutex to get locked"), Poll::Ready(_) => (), }; + match fut4.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(_) => (), + }; + } + + #[test] + fn new_waiters_on_fair_mutex_cant_acquire_future_while_one_task_is_notified() { + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, true); + + // Lock the mutex + let mut guard1 = lock.try_write().unwrap(); + + let mut fut2 = Box::pin(lock.write()); + let mut fut3 = Box::pin(lock.upgradable_read()); + let mut fut4 = Box::pin(lock.read()); + let mut fut5 = Box::pin(lock.write()); + + assert_!(fut2.as_mut().poll(cx).is_pending()); + assert_!(fut3.as_mut().poll(cx).is_pending()); + assert_!(fut4.as_mut().poll(cx).is_pending()); + + // Notify fut2. + drop(guard1); + assert_eq_!(count, 1); + + // Try to steal the lock. + assert_!(fut3.as_mut().poll(cx).is_pending()); + assert_!(fut4.as_mut().poll(cx).is_pending()); + assert_!(fut5.as_mut().poll(cx).is_pending()); + + match fut2.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(_) => {} + }; + + // Now fut3 & fut4 should have been signaled and be lockable + assert_eq_!(count, 3); + + // Try to steal the lock. + assert_!(fut5.as_mut().poll(cx).is_pending()); + + match fut3.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(_) => {} + }; + match fut4.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(_) => {} + }; + + // Now fut3 should have been signaled and be lockable + assert_eq_!(count, 4); + + // Try to steal the lock. + match fut5.as_mut().poll(cx) { + Poll::Pending => panic_!("Expect mutex to get locked"), + Poll::Ready(_) => {} + }; } }; } From 516baaa84cc23b3a70febc57e5a8a40df889ae3a Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 00:08:35 -0400 Subject: [PATCH 19/27] Add test to make sure context is updated --- tests/rwlock.rs | 192 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 128 insertions(+), 64 deletions(-) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index e3fbf7b..ef998d9 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -294,10 +294,7 @@ macro_rules! gen_rwlock_tests { drop(guard); assert_eq_!(count, 1); - match fut.as_mut().poll(cx) { - Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(_) => (), - }; + assert_!(fut.as_mut().poll(cx).is_ready()); assert_!(fut.as_mut().is_terminated()); } } @@ -327,16 +324,10 @@ macro_rules! gen_rwlock_tests { drop(guard); assert_eq_!(count, 2); - match fut1.as_mut().poll(cx) { - Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(_) => (), - }; + assert_!(fut1.as_mut().poll(cx).is_ready()); assert_!(fut1.as_mut().is_terminated()); - match fut2.as_mut().poll(cx) { - Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(_) => (), - }; + assert_!(fut2.as_mut().poll(cx).is_ready()); assert_!(fut2.as_mut().is_terminated()); } } @@ -353,10 +344,7 @@ macro_rules! gen_rwlock_tests { { let fut = lock.read(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(_) => (), - }; + assert_!(fut.as_mut().poll(cx).is_ready()); assert_!(fut.as_mut().is_terminated()); let _ = fut.as_mut().poll(cx); @@ -376,10 +364,7 @@ macro_rules! gen_rwlock_tests { { let fut = lock.write(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(_) => (), - }; + assert_!(fut.as_mut().poll(cx).is_ready()); assert_!(fut.as_mut().is_terminated()); let _ = fut.as_mut().poll(cx); @@ -399,10 +384,7 @@ macro_rules! gen_rwlock_tests { { let fut = lock.upgradable_read(); pin_mut!(fut); - match fut.as_mut().poll(cx) { - Poll::Pending => panic_!("rwlock busy"), - Poll::Ready(_) => (), - }; + assert_!(fut.as_mut().poll(cx).is_ready()); assert_!(fut.as_mut().is_terminated()); let _ = fut.as_mut().poll(cx); @@ -510,10 +492,7 @@ macro_rules! gen_rwlock_tests { drop(guard6); assert_eq_!(count, 7); - match write_fut2.as_mut().poll(cx) { - Poll::Pending => panic_!("busy rwlock"), - Poll::Ready(_) => (), - }; + assert_!(write_fut2.as_mut().poll(cx).is_ready()); assert_eq_!(count, 7); } } @@ -551,10 +530,7 @@ macro_rules! gen_rwlock_tests { assert_eq!(count, 1); // Unlock - mutex should be available again - match fut4.as_mut().poll(cx) { - Poll::Pending => panic!("Expect mutex to get locked"), - Poll::Ready(_) => (), - }; + assert_!(fut4.as_mut().poll(cx).is_ready()); } } @@ -620,16 +596,13 @@ macro_rules! gen_rwlock_tests { drop(fut6); assert_eq_!(count, 6); - match fut7.as_mut().poll(cx) { - Poll::Pending => panic!("lock busy"), - Poll::Ready(_) => (), - }; + assert_!(fut7.as_mut().poll(cx).is_ready()); } } #[test] - fn new_waiters_on_unfair_lock_can_acquire_future_while_one_task_is_notified() { + fn new_waiters_on_unfair_lock_can_acquire_future_while_another_task_is_notified() { let (waker, count) = new_count_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, false); @@ -682,18 +655,56 @@ macro_rules! gen_rwlock_tests { drop(guard2); assert_eq_!(count, 5); - match fut3.as_mut().poll(cx) { - Poll::Pending => panic_!("Expect mutex to get locked"), - Poll::Ready(_) => (), - }; - match fut4.as_mut().poll(cx) { - Poll::Pending => panic_!("Expect mutex to get locked"), - Poll::Ready(_) => (), - }; + assert_!(fut3.as_mut().poll(cx).is_ready()); + assert_!(fut4.as_mut().poll(cx).is_ready()); } #[test] - fn new_waiters_on_fair_mutex_cant_acquire_future_while_one_task_is_notified() { + fn new_waiters_on_fair_mutex_cant_acquire_future_while_another_task_is_notified() { + let (waker, count) = new_count_waker(); + let cx = &mut Context::from_waker(&waker); + let lock = $rwlock_type::new(5, true); + + // Lock the mutex + let mut guard1 = lock.try_write().unwrap(); + + let mut fut2 = Box::pin(lock.write()); + let mut fut3 = Box::pin(lock.upgradable_read()); + let mut fut4 = Box::pin(lock.read()); + let mut fut5 = Box::pin(lock.write()); + + assert_!(fut2.as_mut().poll(cx).is_pending()); + + // Notify fut2. + drop(guard1); + assert_eq_!(count, 1); + + // Try to steal the lock. + assert_!(fut3.as_mut().poll(cx).is_pending()); + assert_!(fut4.as_mut().poll(cx).is_pending()); + assert_!(fut5.as_mut().poll(cx).is_pending()); + + // Take the lock. + assert_!(fut2.as_mut().poll(cx).is_ready()); + + // Now fut3 & fut4 should have been signaled and be lockable + assert_eq_!(count, 3); + + // Try to steal the lock. + assert_!(fut5.as_mut().poll(cx).is_pending()); + + // Take the lock. + assert_!(fut3.as_mut().poll(cx).is_ready()); + assert_!(fut4.as_mut().poll(cx).is_ready()); + + // Now fut5 should have been signaled and be lockable + assert_eq_!(count, 4); + + assert_!(fut5.as_mut().poll(cx).is_ready()); + } + + #[test] + fn waiters_on_fair_mutex_cant_acquire_future_while_another_task_is_notified() { let (waker, count) = new_count_waker(); let cx = &mut Context::from_waker(&waker); let lock = $rwlock_type::new(5, true); @@ -719,10 +730,8 @@ macro_rules! gen_rwlock_tests { assert_!(fut4.as_mut().poll(cx).is_pending()); assert_!(fut5.as_mut().poll(cx).is_pending()); - match fut2.as_mut().poll(cx) { - Poll::Pending => panic_!("Expect mutex to get locked"), - Poll::Ready(_) => {} - }; + // Take the lock. + assert_!(fut2.as_mut().poll(cx).is_ready()); // Now fut3 & fut4 should have been signaled and be lockable assert_eq_!(count, 3); @@ -730,23 +739,78 @@ macro_rules! gen_rwlock_tests { // Try to steal the lock. assert_!(fut5.as_mut().poll(cx).is_pending()); - match fut3.as_mut().poll(cx) { - Poll::Pending => panic_!("Expect mutex to get locked"), - Poll::Ready(_) => {} - }; - match fut4.as_mut().poll(cx) { - Poll::Pending => panic_!("Expect mutex to get locked"), - Poll::Ready(_) => {} - }; + // Take the lock. + assert_!(fut3.as_mut().poll(cx).is_ready()); + assert_!(fut4.as_mut().poll(cx).is_ready()); - // Now fut3 should have been signaled and be lockable + // Now fut5 should have been signaled and be lockable assert_eq_!(count, 4); - // Try to steal the lock. - match fut5.as_mut().poll(cx) { - Poll::Pending => panic_!("Expect mutex to get locked"), - Poll::Ready(_) => {} - }; + // Take the lock. + assert_!(fut5.as_mut().poll(cx).is_ready()); + } + + #[test] + fn poll_from_multiple_executors() { + for is_fair in &[true, false] { + let (waker_1, count_1) = new_count_waker(); + let (waker_2, count_2) = new_count_waker(); + let lock = $rwlock_type::new(5, *is_fair); + + // Lock the mutex + let mut guard1 = lock.try_write().unwrap(); + *guard1 = 27; + + let cx_1 = &mut Context::from_waker(&waker_1); + let cx_2 = &mut Context::from_waker(&waker_2); + + // Wait the read lock and poll using 2 different contexts. + let fut1 = lock.read(); + pin_mut!(fut1); + + assert_!(fut1.as_mut().poll(cx_1).is_pending()); + assert_!(fut1.as_mut().poll(cx_2).is_pending()); + + // Make sure the context was updated properly. + drop(guard1); + assert_eq_!(count_1, 0); + assert_eq_!(count_2, 1); + + let guard2 = match fut1.as_mut().poll(cx_2) { + Poll::Pending => panic_!("busy lock"), + Poll::Ready(guard) => guard, + }; + assert_!(fut1.as_mut().is_terminated()); + + // Wait for the write lock and poll using 2 different contexts. + let fut2 = lock.write(); + pin_mut!(fut2); + assert_!(fut2.as_mut().poll(cx_1).is_pending()); + assert_!(fut2.as_mut().poll(cx_2).is_pending()); + + // Make sure the context was updated properly. + drop(guard2); + assert_eq_!(count_1, 0); + assert_eq_!(count_2, 2); + + let guard3 = match fut2.as_mut().poll(cx_2) { + Poll::Pending => panic_!("busy lock"), + Poll::Ready(guard) => guard, + }; + + // Wait for the upgradable_read lock and poll using 2 different contexts. + let fut3 = lock.upgradable_read(); + pin_mut!(fut3); + assert_!(fut3.as_mut().poll(cx_1).is_pending()); + assert_!(fut3.as_mut().poll(cx_2).is_pending()); + + // Make sure the context was updated properly. + drop(guard3); + assert_eq_!(count_1, 0); + assert_eq_!(count_2, 3); + + assert_!(fut3.as_mut().poll(cx_2).is_ready()); + } } }; } From 28bff8c138bc83bc55e955c7f9a4d98cc589a334 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 00:24:24 -0400 Subject: [PATCH 20/27] Add priority test for upgrade --- src/intrusive_double_linked_list.rs | 19 ++++++++++++ src/sync/rwlock.rs | 45 +++++++++++++++++------------ tests/rwlock.rs | 37 ++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 18 deletions(-) diff --git a/src/intrusive_double_linked_list.rs b/src/intrusive_double_linked_list.rs index 31dd786..7b01644 100644 --- a/src/intrusive_double_linked_list.rs +++ b/src/intrusive_double_linked_list.rs @@ -81,6 +81,25 @@ impl LinkedList { } } + /// Adds a node at the back of the linked list. + /// + /// Safety: This function is only safe as long as `node` is guaranteed to + /// get removed from the list before it gets moved or dropped. + /// In addition to this `node` may not be added to another other list before + /// it is removed from the current one. + pub unsafe fn add_back(&mut self, node: &mut ListNode) { + node.next = None; + node.prev = self.tail; + match self.tail { + Some(mut tail) => tail.as_mut().next = Some(node.into()), + None => {} + }; + self.tail = Some(node.into()); + if self.head.is_none() { + self.head = Some(node.into()); + } + } + /// Returns a reference to the first node in the linked list /// The function is only safe as long as valid pointers are stored inside /// the linked list. diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 87f8c8a..68b24a8 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -233,10 +233,7 @@ impl MutexState { fn unlock_upgrade_read_for_upgrade(&mut self) { self.has_upgrade_read = false; self.nb_reads -= 1; - - if self.nb_reads == 0 && self.is_fair { - self.wakeup_any_waiters(); - } + // We don't wakeup anyone because the upgrade has priority. } fn wakeup_any_waiters(&mut self) { @@ -314,16 +311,9 @@ impl MutexState { /// Returns true if the access is obtained. fn try_upgrade_read_sync(&mut self) -> bool { // The lock can only be obtained synchronously if - // - has no write // - has 1 read (the caller) - // - the Semaphore is either not fair, or there are no waiting writes. debug_assert!(self.has_upgrade_read); - if !self.has_write - && self.nb_reads == 1 - && (!self.is_fair - || (self.nb_waiting_writes == 0 - && self.nb_waiting_upgrade_reads == 0)) - { + if self.nb_reads == 1 { self.has_write = true; self.nb_reads -= 1; self.has_upgrade_read = false; @@ -350,9 +340,17 @@ impl MutexState { /// Safety: This function is only safe as long as `node` is guaranteed to /// get removed from the list before it gets moved or dropped. /// In addition to this `node` may not be added to another other list before it is removed from the current one. - unsafe fn add_write(&mut self, wait_node: &mut ListNode) { + unsafe fn add_write( + &mut self, + wait_node: &mut ListNode, + has_priority: bool, + ) { debug_assert_eq!(wait_node.kind, EntryKind::Write); - self.waiters.add_front(wait_node); + if !has_priority { + self.waiters.add_front(wait_node); + } else { + self.waiters.add_back(wait_node); + } self.nb_waiting_writes += 1; } @@ -466,6 +464,7 @@ impl MutexState { &mut self, wait_node: &mut ListNode, cx: &mut Context<'_>, + has_priority: bool, ) -> Poll<()> { match wait_node.state { PollState::New => { @@ -478,7 +477,7 @@ impl MutexState { // Add the task to the wait queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_write(wait_node); + self.add_write(wait_node, has_priority); Poll::Pending } } @@ -533,7 +532,7 @@ impl MutexState { // Add to queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_write(wait_node); + self.add_write(wait_node, has_priority); Poll::Pending } } @@ -957,6 +956,9 @@ pub struct GenericRwLockWriteFuture<'a, MutexType: RawMutex, T: 'a> { mutex: Option<&'a GenericRwLock>, /// Node for waiting at the mutex wait_node: ListNode, + /// Whether this is a upgradable_read lock that is being upgraded, + /// thus has priority over the other writers. + has_priority: bool, } // Safety: Futures can be sent between threads as long as the underlying @@ -992,8 +994,13 @@ impl<'a, MutexType: RawMutex, T> Future .expect("polled GenericRwLockWriteFuture after completion"); let mut mutex_state = mutex.state.lock(); - let poll_res = - unsafe { mutex_state.try_lock_write(&mut mut_self.wait_node, cx) }; + let poll_res = unsafe { + mutex_state.try_lock_write( + &mut mut_self.wait_node, + cx, + mut_self.has_priority, + ) + }; match poll_res { Poll::Pending => Poll::Pending, @@ -1050,6 +1057,7 @@ impl<'a, MutexType: RawMutex, T> GenericRwLockWriteFuture:: { mutex: Some(mutex), wait_node: ListNode::new(Entry::new(EntryKind::Write)), + has_priority: true, } } @@ -1265,6 +1273,7 @@ impl GenericRwLock { GenericRwLockWriteFuture:: { mutex: Some(&self), wait_node: ListNode::new(Entry::new(EntryKind::Write)), + has_priority: false, } } diff --git a/tests/rwlock.rs b/tests/rwlock.rs index ef998d9..32f4fbd 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -812,6 +812,43 @@ macro_rules! gen_rwlock_tests { assert_!(fut3.as_mut().poll(cx_2).is_ready()); } } + + #[test] + fn upgrade_guard_has_priority_over_writer() { + for is_fair in &[true, false] { + let (waker, count) = new_count_waker(); + let lock = $rwlock_type::new(5, *is_fair); + + // Acquire the upgradable lock. + let mut guard1 = lock.try_read().unwrap(); + let mut guard2 = lock.try_upgradable_read().unwrap(); + + let cx = &mut Context::from_waker(&waker); + + // Wait for the write lock. + let fut3 = lock.write(); + pin_mut!(fut3); + assert_!(fut3.as_mut().poll(cx).is_pending()); + + // We can't upgrade because of the other reader. + let guard2 = guard2.try_upgrade().unwrap_err(); + + // Add contention for the write lock. + let fut4 = guard2.upgrade(); + pin_mut!(fut4); + assert_!(fut4.as_mut().poll(cx).is_pending()); + + assert_eq_!(count, 0); + // This should wakeup the upgrading upgradable_read because + // it has priority. + drop(guard1); + assert_eq_!(count, 1); + + assert_!(fut4.as_mut().poll(cx).is_ready()); + assert_eq_!(count, 2); + assert_!(fut3.as_mut().poll(cx).is_ready()); + } + } }; } From 7502d4193af404a2e9afa37ec32c0219dc24a122 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 00:27:28 -0400 Subject: [PATCH 21/27] Add synchronous test for upgrade --- tests/rwlock.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index 32f4fbd..7b5e181 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -849,6 +849,28 @@ macro_rules! gen_rwlock_tests { assert_!(fut3.as_mut().poll(cx).is_ready()); } } + + #[test] + fn upgrade_guard_sync_contention() { + for is_fair in &[true, false] { + let (waker, count) = new_count_waker(); + let lock = $rwlock_type::new(5, *is_fair); + + // Acquire the upgradable lock. + let mut guard1 = lock.try_read().unwrap(); + let mut guard2 = lock.try_upgradable_read().unwrap(); + + // We can't upgrade because of the oother reader. + let guard2 = guard2.try_upgrade().unwrap_err(); + drop(guard1); + + // Can't acquire a write lock because of the upgradable_read. + assert_!(lock.try_write().is_none()); + + // Now we can upgrade. + let _guard3 = guard2.try_upgrade().unwrap(); + } + } }; } From 8577f63a17f7fd6f907c2e24f8f258738d5ffe3e Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 00:28:46 -0400 Subject: [PATCH 22/27] Fix clippy issues --- src/sync/rwlock.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 68b24a8..3bf3586 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -106,12 +106,10 @@ impl FilterWaiters { } else { ControlFlow::Stop } + } else if self.remove_notified { + ControlFlow::RemoveAndContinue } else { - if self.remove_notified { - ControlFlow::RemoveAndContinue - } else { - ControlFlow::Continue - } + ControlFlow::Continue } } EntryKind::UpgradeRead => { @@ -132,12 +130,10 @@ impl FilterWaiters { } else { ControlFlow::Stop } + } else if self.remove_notified { + ControlFlow::RemoveAndContinue } else { - if self.remove_notified { - ControlFlow::RemoveAndContinue - } else { - ControlFlow::Continue - } + ControlFlow::Continue } } } From d6f4a822895369baf840be02d81ae755998cd876 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 00:30:33 -0400 Subject: [PATCH 23/27] Improve test doc --- tests/rwlock.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/rwlock.rs b/tests/rwlock.rs index 7b5e181..fd400a8 100644 --- a/tests/rwlock.rs +++ b/tests/rwlock.rs @@ -844,8 +844,11 @@ macro_rules! gen_rwlock_tests { drop(guard1); assert_eq_!(count, 1); + // We got the write lock first because of priority. assert_!(fut4.as_mut().poll(cx).is_ready()); assert_eq_!(count, 2); + + // Make sure the lock is released properly. assert_!(fut3.as_mut().poll(cx).is_ready()); } } From e41f7d288e2a8259a6aede8d6a8f70d2a291b58f Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 00:52:12 -0400 Subject: [PATCH 24/27] Improve doc --- src/sync/rwlock.rs | 114 +++++++++++++++++++++++++++++++-------------- 1 file changed, 79 insertions(+), 35 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 3bf3586..247058d 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -44,6 +44,9 @@ enum EntryKind { Read, /// A upgradable shared read access. + /// + /// While only one `UpgradeRead` guard can be open at a time, it can be + /// open alongside any number of `Read` guards. UpgradeRead, } @@ -81,6 +84,9 @@ impl Entry { } } +/// Keeps track of the hypothetical future state of the `MutexState` +/// if all the notified waiters were to acquire their respective +/// locks. #[derive(Debug)] struct FilterWaiters { nb_waiting_reads: usize, @@ -91,16 +97,23 @@ struct FilterWaiters { } impl FilterWaiters { + /// Wakeup as many waiters without contention. fn filter(&mut self, entry: &mut ListNode) -> ControlFlow { match entry.kind { EntryKind::Read => { + // Since a writer lock is exclusive, it will end the + // iteration, implying that there is not writer lock. + // Therefore we can notify every read lock waiter. entry.notify(); + debug_assert!(!self.has_write); self.nb_reads += 1; self.nb_waiting_reads -= 1; + if self.nb_waiting_reads == 0 && (self.has_upgrade_read || self.nb_waiting_upgrade_reads == 0) { + // This is the last reader. if self.remove_notified { ControlFlow::RemoveAndStop } else { @@ -117,14 +130,19 @@ impl FilterWaiters { if self.nb_waiting_reads > 0 { ControlFlow::Continue } else { + // This was the last reader. ControlFlow::Stop } } else { + // Like with the `Read` variant, we know that the lock + // is currently shared. + debug_assert!(!self.has_write); entry.notify(); self.has_upgrade_read = true; self.nb_reads += 1; self.nb_waiting_upgrade_reads -= 1; if self.nb_waiting_reads == 0 { + // This was the last reader. if self.remove_notified { ControlFlow::RemoveAndStop } else { @@ -139,6 +157,8 @@ impl FilterWaiters { } EntryKind::Write => { if self.nb_reads == 0 { + // We have the exclusive write lock so we end + // the iteration. entry.notify(); if self.remove_notified { ControlFlow::RemoveAndStop @@ -151,6 +171,7 @@ impl FilterWaiters { { ControlFlow::Continue } else { + // Iteration would be a waste of time. ControlFlow::Stop } } @@ -160,13 +181,29 @@ impl FilterWaiters { /// Internal state of the `Mutex` struct MutexState { + /// Whether this is a fair mutex, meaning the queue order + /// is respected. is_fair: bool, + + /// The current number of open read lock guards. nb_reads: usize, + + /// Whether a exclusive upgradable read guard is open. has_upgrade_read: bool, + + /// Whether a exclusive writer guard is open. has_write: bool, + + /// The waiter queue. waiters: LinkedList, + + /// The number of write lock waiters in the queue. nb_waiting_writes: usize, + + /// The number of read lock waiters in the queue. nb_waiting_reads: usize, + + /// The number of upgradable_read lock waiters in the queue. nb_waiting_upgrade_reads: usize, } @@ -185,7 +222,7 @@ impl MutexState { } /// Reduce the number of reads on the lock by one, waking - /// up a write if needed. + /// up waiters if needed. /// /// If the Mutex is not fair, removes the woken up node from /// the wait queue @@ -223,15 +260,16 @@ impl MutexState { } /// Release the upgradable reader lock to upgrade - /// it into a writer lock, waking up entries if needed. - /// - /// If the Mutex is not fair does not wakeup anyone. + /// it into a writer lock. fn unlock_upgrade_read_for_upgrade(&mut self) { self.has_upgrade_read = false; self.nb_reads -= 1; // We don't wakeup anyone because the upgrade has priority. } + /// Wakeup waiters from back to front. + /// + /// If the mutex is unfair, notified entries are removed from the queue. fn wakeup_any_waiters(&mut self) { let mut filter = FilterWaiters { nb_waiting_reads: self.nb_waiting_reads, @@ -302,7 +340,8 @@ impl MutexState { } } - /// Attempt to gain exclusive write access. + /// Attempt to gain exclusive write access by upgrading a upgradable_read + /// lock. /// /// Returns true if the access is obtained. fn try_upgrade_read_sync(&mut self) -> bool { @@ -319,24 +358,28 @@ impl MutexState { } } - /// Add a read to the wait queue. + /// Add a read lock waiter to the wait queue. /// /// Safety: This function is only safe as long as `node` is guaranteed to /// get removed from the list before it gets moved or dropped. /// In addition to this `node` may not be added to another other list before /// it is removed from the current one. - unsafe fn add_read(&mut self, wait_node: &mut ListNode) { + unsafe fn add_read_waiter(&mut self, wait_node: &mut ListNode) { debug_assert_eq!(wait_node.kind, EntryKind::Read); self.waiters.add_front(wait_node); self.nb_waiting_reads += 1; } - /// Add a write to the wait queue. + /// Add a write lock waiter to the wait queue. + /// + /// If the write has priority, it will be added to the back instead + /// of the front, meaning it will wakeup first. /// /// Safety: This function is only safe as long as `node` is guaranteed to /// get removed from the list before it gets moved or dropped. - /// In addition to this `node` may not be added to another other list before it is removed from the current one. - unsafe fn add_write( + /// In addition to this `node` may not be added to another other list before + /// it is removed from the current one. + unsafe fn add_write_waiter( &mut self, wait_node: &mut ListNode, has_priority: bool, @@ -350,13 +393,16 @@ impl MutexState { self.nb_waiting_writes += 1; } - /// Add a write to the wait queue. + /// Add a upgradable_read lock waiter to the wait queue. /// /// Safety: This function is only safe as long as `node` is guaranteed to /// get removed from the list before it gets moved or dropped. /// In addition to this `node` may not be added to another other list before /// it is removed from the current one. - unsafe fn add_upgrade_read(&mut self, wait_node: &mut ListNode) { + unsafe fn add_upgrade_read_waiter( + &mut self, + wait_node: &mut ListNode, + ) { debug_assert_eq!(wait_node.kind, EntryKind::UpgradeRead); self.waiters.add_front(wait_node); self.nb_waiting_upgrade_reads += 1; @@ -384,7 +430,7 @@ impl MutexState { // Add the task to the wait queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_read(wait_node); + self.add_read_waiter(wait_node); Poll::Pending } } @@ -439,7 +485,7 @@ impl MutexState { // Add to queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_read(wait_node); + self.add_read_waiter(wait_node); Poll::Pending } } @@ -473,7 +519,7 @@ impl MutexState { // Add the task to the wait queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_write(wait_node, has_priority); + self.add_write_waiter(wait_node, has_priority); Poll::Pending } } @@ -528,7 +574,7 @@ impl MutexState { // Add to queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_write(wait_node, has_priority); + self.add_write_waiter(wait_node, has_priority); Poll::Pending } } @@ -561,7 +607,7 @@ impl MutexState { // Add the task to the wait queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_upgrade_read(wait_node); + self.add_upgrade_read_waiter(wait_node); Poll::Pending } } @@ -618,7 +664,7 @@ impl MutexState { // Add to queue wait_node.task = Some(cx.waker().clone()); wait_node.state = PollState::Waiting; - self.add_upgrade_read(wait_node); + self.add_upgrade_read_waiter(wait_node); Poll::Pending } } @@ -665,7 +711,7 @@ impl MutexState { self.nb_waiting_upgrade_reads -= 1; } - /// Removes the read from the wait list. + /// Removes the read lock waiter from the wait list. /// /// This function is only safe as long as the reference that is passed here /// equals the reference/address under which the waiter was added. @@ -702,7 +748,7 @@ impl MutexState { } } - /// Removes the write from the wait list. + /// Removes the write lock waiter from the wait list. /// /// This function is only safe as long as the reference that is passed here /// equals the reference/address under which the waiter was added. @@ -739,7 +785,7 @@ impl MutexState { } } - /// Removes the upgrade_read from the wait list. + /// Removes the upgrade_read lock waiter from the wait list. /// /// This function is only safe as long as the reference that is passed here /// equals the reference/address under which the waiter was added. @@ -1033,9 +1079,9 @@ impl<'a, MutexType: RawMutex, T> Drop } } -/// An RAII guard returned by the `write` and `try_write` methods. -/// When this structure is dropped (falls out of scope), the -/// exclusive write access will be released. +/// An RAII guard returned by the `upgradable_read` and `try_upgradable_read` +/// methods. When this structure is dropped (falls out of scope), the +/// shared read access and exclusive upgrade_read access will be released. pub struct GenericRwLockUpgradableReadGuard<'a, MutexType: RawMutex, T: 'a> { /// The Mutex which is associated with this Guard mutex: Option<&'a GenericRwLock>, @@ -1044,7 +1090,7 @@ pub struct GenericRwLockUpgradableReadGuard<'a, MutexType: RawMutex, T: 'a> { impl<'a, MutexType: RawMutex, T> GenericRwLockUpgradableReadGuard<'a, MutexType, T> { - /// Asynchrousnly upgrade the shared read lock into an exclusive write lock. + /// Asynchrously upgrade the shared read lock into an exclusive write lock. pub fn upgrade(mut self) -> GenericRwLockWriteFuture<'a, MutexType, T> { let mutex = self.mutex.take().unwrap(); let mut state = mutex.state.lock(); @@ -1057,8 +1103,8 @@ impl<'a, MutexType: RawMutex, T> } } - /// Atomically upgrade the shared read lock into an exclusive write lock, - /// blocking the current thread. + /// Attempt to atomically upgrade the shared read lock into an + /// exclusive write lock. pub fn try_upgrade( mut self, ) -> Result, Self> { @@ -1107,7 +1153,7 @@ unsafe impl Sync { } -/// A future which resolves when exclusive write access has been successfully acquired. +/// A future which resolves when upgrade_read lock access has been successfully acquired. #[must_use = "futures do nothing unless polled"] pub struct GenericRwLockUpgradableReadFuture<'a, MutexType: RawMutex, T: 'a> { /// The Mutex which should get locked trough this Future @@ -1223,12 +1269,10 @@ impl core::fmt::Debug impl GenericRwLock { /// Creates a new futures-aware mutex. /// - /// `is_fair` defines whether the `Mutex` should behave be fair regarding the - /// order of waiters. A fair `Mutex` will only allow the first waiter which - /// tried to lock but failed to lock the `Mutex` once it's available again. - /// Other waiters must wait until either this locking attempt completes, and - /// the `Mutex` gets unlocked again, or until the `MutexLockFuture` which - /// tried to gain the lock is dropped. + /// `is_fair` defines whether the `RwLock` should behave be fair regarding the + /// order of waiters. A fair `RwLock` will respect the priority queue + /// so that older waiters will be prioritized over newer waiters + /// when polling to acquire their lock. pub fn new(value: T, is_fair: bool) -> GenericRwLock { GenericRwLock:: { value: UnsafeCell::new(value), @@ -1318,7 +1362,7 @@ impl GenericRwLock { } } - /// Returns whether the rwlock is locked in exclusive access. + /// Returns whether the `RwLock` is locked in exclusive access. pub fn is_exclusive(&self) -> bool { self.state.lock().has_write } From 8dc7d6f98cfb47bc2dda7d41e031ee014a579ffa Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 01:20:06 -0400 Subject: [PATCH 25/27] Rework waiter notification method for clarity --- src/sync/rwlock.rs | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 247058d..565d96f 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -105,7 +105,6 @@ impl FilterWaiters { // iteration, implying that there is not writer lock. // Therefore we can notify every read lock waiter. entry.notify(); - debug_assert!(!self.has_write); self.nb_reads += 1; self.nb_waiting_reads -= 1; @@ -136,7 +135,6 @@ impl FilterWaiters { } else { // Like with the `Read` variant, we know that the lock // is currently shared. - debug_assert!(!self.has_write); entry.notify(); self.has_upgrade_read = true; self.nb_reads += 1; @@ -228,9 +226,7 @@ impl MutexState { /// the wait queue fn unlock_read(&mut self) { self.nb_reads -= 1; - if self.nb_reads == 0 { - self.wakeup_any_waiters(); - } + self.wakeup_waiters_if_needed(); } /// Release the exclusive writer lock, waking up entries @@ -242,7 +238,7 @@ impl MutexState { self.has_write = false; debug_assert_eq!(self.nb_reads, 0); - self.wakeup_any_waiters() + self.wakeup_waiters_if_needed() } /// Release the upgradable reader lock, waking up entries @@ -253,10 +249,7 @@ impl MutexState { fn unlock_upgrade_read(&mut self) { self.has_upgrade_read = false; self.nb_reads -= 1; - - if self.nb_reads == 0 || self.nb_waiting_upgrade_reads > 0 { - self.wakeup_any_waiters(); - } + self.wakeup_waiters_if_needed(); } /// Release the upgradable reader lock to upgrade @@ -267,10 +260,32 @@ impl MutexState { // We don't wakeup anyone because the upgrade has priority. } + /// Returns the number of read waiters that could acquire + /// the read lock immediately. + fn nb_immediate_read_waiters(&self) -> usize { + if self.has_upgrade_read { + // We don't count the nb_waiting_upgrade_reads because + // they cannot immediately acquire the read lock. + self.nb_waiting_reads + } else { + self.nb_waiting_reads + self.nb_waiting_upgrade_reads + } + } + /// Wakeup waiters from back to front. /// /// If the mutex is unfair, notified entries are removed from the queue. - fn wakeup_any_waiters(&mut self) { + fn wakeup_waiters_if_needed(&mut self) { + // We never need to wakeup waiters if + // - an exclusive write lock is held. + // - the queue is empty. + // - there are readers but no immediate read waiters. + if self.has_write + || self.waiters.is_empty() + || (self.nb_reads > 0 && self.nb_immediate_read_waiters() == 0) + { + return; + } let mut filter = FilterWaiters { nb_waiting_reads: self.nb_waiting_reads, nb_waiting_upgrade_reads: self.nb_waiting_upgrade_reads, @@ -735,7 +750,7 @@ impl MutexState { wait_node.state = PollState::Done; // Since the task was notified but did not lock the Mutex, // another task gets the chance to run. - self.wakeup_any_waiters(); + self.wakeup_waiters_if_needed(); } PollState::Waiting => { // Remove the Entry from the linked list @@ -772,7 +787,7 @@ impl MutexState { wait_node.state = PollState::Done; // Since the task was notified but did not lock the Mutex, // another task gets the chance to run. - self.wakeup_any_waiters(); + self.wakeup_waiters_if_needed(); } PollState::Waiting => { // Remove the Entry from the linked list @@ -809,7 +824,7 @@ impl MutexState { wait_node.state = PollState::Done; // Since the task was notified but did not lock the Mutex, // another task gets the chance to run. - self.wakeup_any_waiters(); + self.wakeup_waiters_if_needed(); } PollState::Waiting => { // Remove the Entry from the linked list From 870bd3fa31481da6107d97aee9ad234dc5ab13b9 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 01:21:40 -0400 Subject: [PATCH 26/27] Fix nb_immediate_read_waiters --- src/sync/rwlock.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index 565d96f..e78d88a 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -263,12 +263,12 @@ impl MutexState { /// Returns the number of read waiters that could acquire /// the read lock immediately. fn nb_immediate_read_waiters(&self) -> usize { - if self.has_upgrade_read { - // We don't count the nb_waiting_upgrade_reads because - // they cannot immediately acquire the read lock. + if self.has_write { + 0 + } else if self.has_upgrade_read { self.nb_waiting_reads } else { - self.nb_waiting_reads + self.nb_waiting_upgrade_reads + self.nb_waiting_reads + 1 } } From a4ef173001dbd1f1bc226b5f3d5a3260ab066563 Mon Sep 17 00:00:00 2001 From: jean-airoldie <25088801+jean-airoldie@users.noreply.github.com> Date: Mon, 25 Mar 2024 01:27:08 -0400 Subject: [PATCH 27/27] Upgrade upgrade doc --- src/sync/rwlock.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index e78d88a..138b3f2 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -1106,6 +1106,9 @@ impl<'a, MutexType: RawMutex, T> GenericRwLockUpgradableReadGuard<'a, MutexType, T> { /// Asynchrously upgrade the shared read lock into an exclusive write lock. + /// + /// Note that this future will have priority over all other competing + /// write futures. pub fn upgrade(mut self) -> GenericRwLockWriteFuture<'a, MutexType, T> { let mutex = self.mutex.take().unwrap(); let mut state = mutex.state.lock();