From b71a601a36c3f52a8618027b9f817ff5524af38a Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Sat, 30 Mar 2019 00:15:55 +0000 Subject: [PATCH] style: Add support for read only SharedRwLocks, which don't need any locking. Differential Revision: https://phabricator.services.mozilla.com/D17185 --- components/style/shared_lock.rs | 59 ++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/components/style/shared_lock.rs b/components/style/shared_lock.rs index 09bfa163503..f708ec58a23 100644 --- a/components/style/shared_lock.rs +++ b/components/style/shared_lock.rs @@ -28,6 +28,10 @@ use std::ptr; /// Servo needs the blocking behavior for its unsynchronized animation setup, /// but that may not be web-compatible and may need to be changed (at which /// point Servo could use AtomicRefCell too). +/// +/// Gecko also needs the ability to have "read only" SharedRwLocks, which are +/// used for objects stored in (read only) shared memory. Attempting to acquire +/// write access to objects protected by a read only SharedRwLock will panic. #[derive(Clone)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] pub struct SharedRwLock { @@ -36,7 +40,7 @@ pub struct SharedRwLock { arc: Arc>, #[cfg(feature = "gecko")] - cell: Arc>, + cell: Option>>, } #[cfg(feature = "gecko")] @@ -61,10 +65,16 @@ impl SharedRwLock { #[cfg(feature = "gecko")] pub fn new() -> Self { SharedRwLock { - cell: Arc::new(AtomicRefCell::new(SomethingZeroSizedButTyped)), + cell: Some(Arc::new(AtomicRefCell::new(SomethingZeroSizedButTyped))), } } + /// Create a new read-only shared lock (gecko). + #[cfg(feature = "gecko")] + pub fn read_only() -> Self { + SharedRwLock { cell: None } + } + /// Wrap the given data to make its access protected by this lock. pub fn wrap(&self, data: T) -> Locked { Locked { @@ -83,7 +93,7 @@ impl SharedRwLock { /// Obtain the lock for reading (gecko). #[cfg(feature = "gecko")] pub fn read(&self) -> SharedRwLockReadGuard { - SharedRwLockReadGuard(self.cell.borrow()) + SharedRwLockReadGuard(self.cell.as_ref().map(|cell| cell.borrow())) } /// Obtain the lock for writing (servo). @@ -96,22 +106,26 @@ impl SharedRwLock { /// Obtain the lock for writing (gecko). #[cfg(feature = "gecko")] pub fn write(&self) -> SharedRwLockWriteGuard { - SharedRwLockWriteGuard(self.cell.borrow_mut()) + SharedRwLockWriteGuard(self.cell.as_ref().unwrap().borrow_mut()) } } /// Proof that a shared lock was obtained for reading (servo). #[cfg(feature = "servo")] pub struct SharedRwLockReadGuard<'a>(&'a SharedRwLock); -/// Proof that a shared lock was obtained for writing (gecko). +/// Proof that a shared lock was obtained for reading (gecko). #[cfg(feature = "gecko")] -pub struct SharedRwLockReadGuard<'a>(AtomicRef<'a, SomethingZeroSizedButTyped>); +pub struct SharedRwLockReadGuard<'a>(Option>); #[cfg(feature = "servo")] impl<'a> Drop for SharedRwLockReadGuard<'a> { fn drop(&mut self) { // Unsafe: self.lock is private to this module, only ever set after `read()`, // and never copied or cloned (see `compile_time_assert` below). - unsafe { self.0.arc.force_unlock_read() } + if let Some(arc) = self.0.arc { + unsafe { + arc.force_unlock_read(); + } + } } } @@ -149,20 +163,41 @@ impl fmt::Debug for Locked { } impl Locked { + #[cfg(feature = "servo")] + #[inline] + fn is_read_only_lock(&self) -> bool { + false + } + + #[cfg(feature = "gecko")] + #[inline] + fn is_read_only_lock(&self) -> bool { + self.shared_lock.cell.is_none() + } + #[cfg(feature = "servo")] fn same_lock_as(&self, lock: &SharedRwLock) -> bool { Arc::ptr_eq(&self.shared_lock.arc, &lock.arc) } #[cfg(feature = "gecko")] - fn same_lock_as(&self, derefed_guard: &SomethingZeroSizedButTyped) -> bool { - ptr::eq(self.shared_lock.cell.as_ptr(), derefed_guard) + fn same_lock_as(&self, derefed_guard: Option<&SomethingZeroSizedButTyped>) -> bool { + ptr::eq( + self.shared_lock + .cell + .as_ref() + .map(|cell| cell.as_ptr()) + .unwrap_or(ptr::null_mut()), + derefed_guard + .map(|guard| guard as *const _ as *mut _) + .unwrap_or(ptr::null_mut()), + ) } /// Access the data for reading. pub fn read_with<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> &'a T { assert!( - self.same_lock_as(&guard.0), + self.is_read_only_lock() || self.same_lock_as(guard.0.as_ref().map(|r| &**r)), "Locked::read_with called with a guard from an unrelated SharedRwLock" ); let ptr = self.data.get(); @@ -186,8 +221,8 @@ impl Locked { /// Access the data for writing. pub fn write_with<'a>(&'a self, guard: &'a mut SharedRwLockWriteGuard) -> &'a mut T { assert!( - self.same_lock_as(&guard.0), - "Locked::write_with called with a guard from an unrelated SharedRwLock" + !self.is_read_only_lock() && self.same_lock_as(Some(&guard.0)), + "Locked::write_with called with a guard from a read only or unrelated SharedRwLock" ); let ptr = self.data.get();