Replace remutex with parking_lot's ReentrantMutex (#31817)

Many things in Servo depend on `parking_lot`, so we can replace our
homegrown remutex with `parking_lot`'s version.

Fixes #12641.

Cargo.lock

@@ -875,13 +875,13 @@ dependencies = [
  "msg",
  "net",
  "net_traits",
+ "parking_lot",
  "profile_traits",
  "script_layout_interface",
  "script_traits",
  "serde",
  "servo_config",
  "servo_rand",
- "servo_remutex",
  "servo_url",
  "style_traits",
  "webgpu",
@@ -5420,14 +5420,6 @@ dependencies = [
  "uuid",
 ]
 
-[[package]]
-name = "servo_remutex"
-version = "0.0.1"
-dependencies = [
- "lazy_static",
- "log",
-]
-
 [[package]]
 name = "servo_url"
 version = "0.0.1"

components/constellation/Cargo.toml

@@ -35,13 +35,13 @@ metrics = { path = "../metrics" }
 msg = { workspace = true }
 net = { path = "../net" }
 net_traits = { workspace = true }
+parking_lot = { workspace = true }
 profile_traits = { workspace = true }
 script_layout_interface = { workspace = true }
 script_traits = { workspace = true }
 serde = { workspace = true }
 servo_config = { path = "../config" }
 servo_rand = { path = "../rand" }
-servo_remutex = { path = "../remutex" }
 servo_url = { path = "../url" }
 style_traits = { workspace = true }
 webgpu = { path = "../webgpu" }

components/constellation/logging.rs

@@ -11,8 +11,8 @@ use compositing_traits::ConstellationMsg as FromCompositorMsg;
 use crossbeam_channel::Sender;
 use log::{Level, LevelFilter, Log, Metadata, Record};
 use msg::constellation_msg::TopLevelBrowsingContextId;
+use parking_lot::ReentrantMutex;
 use script_traits::{LogEntry, ScriptMsg as FromScriptMsg, ScriptToConstellationChan};
-use servo_remutex::ReentrantMutex;
 
 /// The constellation uses logging to perform crash reporting.
 /// The constellation receives all `warn!`, `error!` and `panic!` messages,
@@ -55,10 +55,7 @@ impl Log for FromScriptLogger {
         if let Some(entry) = log_entry(record) {
             let thread_name = thread::current().name().map(ToOwned::to_owned);
             let msg = FromScriptMsg::LogEntry(thread_name, entry);
-            let chan = self
+            let chan = self.script_to_constellation_chan.lock();
-                .script_to_constellation_chan
-                .lock()
-                .unwrap_or_else(|err| err.into_inner());
             let _ = chan.send(msg);
         }
     }
@@ -97,10 +94,7 @@ impl Log for FromCompositorLogger {
             let top_level_id = TopLevelBrowsingContextId::installed();
             let thread_name = thread::current().name().map(ToOwned::to_owned);
             let msg = FromCompositorMsg::LogEntry(top_level_id, thread_name, entry);
-            let chan = self
+            let chan = self.constellation_chan.lock();
-                .constellation_chan
-                .lock()
-                .unwrap_or_else(|err| err.into_inner());
             let _ = chan.send(msg);
         }
     }

components/remutex/Cargo.toml

@@ -1,17 +0,0 @@
-[package]
-name = "servo_remutex"
-version = "0.0.1"
-authors = ["The Servo Project Developers"]
-license = "MPL-2.0"
-edition = "2018"
-publish = false
-
-[lib]
-name = "servo_remutex"
-path = "lib.rs"
-test = false
-doctest = false
-
-[dependencies]
-lazy_static = { workspace = true }
-log = { workspace = true }

components/remutex/lib.rs

@@ -1,259 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
-
-//! An implementation of re-entrant mutexes.
-//!
-//! Re-entrant mutexes are like mutexes, but where it is expected
-//! that a single thread may own a lock more than once.
-
-//! It provides the same interface as <https://github.com/rust-lang/rust/blob/5edaa7eefd76d4996dcf85dfc1c1a3f737087257/src/libstd/sys_common/remutex.rs>
-//! so if those types are ever exported, we should be able to replace this implemtation.
-
-use std::cell::{Cell, UnsafeCell};
-use std::num::NonZeroUsize;
-use std::ops::Deref;
-use std::sync::atomic::{AtomicUsize, Ordering};
-use std::sync::{LockResult, Mutex, MutexGuard, PoisonError, TryLockError, TryLockResult};
-
-use log::trace;
-
-/// A type for thread ids.
-
-// TODO: can we use the thread-id crate for this?
-
-#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
-pub struct ThreadId(NonZeroUsize);
-
-lazy_static::lazy_static! {
-    static ref THREAD_COUNT: AtomicUsize = AtomicUsize::new(1);
-}
-
-impl ThreadId {
-    #[allow(unsafe_code)]
-    fn new() -> ThreadId {
-        let number = THREAD_COUNT.fetch_add(1, Ordering::SeqCst);
-        ThreadId(NonZeroUsize::new(number).unwrap())
-    }
-    pub fn current() -> ThreadId {
-        THREAD_ID.with(|tls| *tls)
-    }
-}
-
-thread_local! { static THREAD_ID: ThreadId = ThreadId::new() }
-
-/// A type for atomic storage of thread ids.
-#[derive(Debug)]
-pub struct AtomicOptThreadId(AtomicUsize);
-
-impl Default for AtomicOptThreadId {
-    fn default() -> Self {
-        AtomicOptThreadId(AtomicUsize::new(0))
-    }
-}
-
-impl AtomicOptThreadId {
-    pub fn store(&self, value: Option<ThreadId>, ordering: Ordering) {
-        let number = value.map(|id| id.0.get()).unwrap_or(0);
-        self.0.store(number, ordering);
-    }
-    #[allow(unsafe_code)]
-    pub fn load(&self, ordering: Ordering) -> Option<ThreadId> {
-        let number = self.0.load(ordering);
-        NonZeroUsize::new(number).map(ThreadId)
-    }
-}
-
-/// A type for hand-over-hand mutexes.
-///
-/// These support `lock` and `unlock` functions. `lock` blocks waiting to become the
-/// mutex owner. `unlock` can only be called by the lock owner, and panics otherwise.
-/// They have the same happens-before and poisoning semantics as `Mutex`.
-
-// TODO: Can we use `raw_lock` and `raw_unlock` from `parking_lot`'s `Mutex` for this?
-
-pub struct HandOverHandMutex {
-    mutex: Mutex<()>,
-    owner: AtomicOptThreadId,
-    guard: UnsafeCell<Option<MutexGuard<'static, ()>>>,
-}
-
-impl Default for HandOverHandMutex {
-    fn default() -> Self {
-        Self {
-            mutex: Mutex::new(()),
-            owner: AtomicOptThreadId::default(),
-            guard: UnsafeCell::new(None),
-        }
-    }
-}
-
-impl HandOverHandMutex {
-    #[allow(unsafe_code)]
-    unsafe fn set_guard_and_owner<'a>(&'a self, guard: MutexGuard<'a, ()>) {
-        // The following two lines allow us to unsafely store
-        // Some(guard): Option<MutexGuard<'a, ()>
-        // in self.guard, even though its contents are Option<MutexGuard<'static, ()>>,
-        // that is the lifetime is 'a not 'static.
-        let guard_ptr = &mut *(self.guard.get() as *mut u8 as *mut Option<MutexGuard<'a, ()>>);
-        *guard_ptr = Some(guard);
-        self.owner
-            .store(Some(ThreadId::current()), Ordering::Relaxed);
-    }
-    #[allow(unsafe_code)]
-    unsafe fn unset_guard_and_owner(&self) {
-        let guard_ptr = &mut *self.guard.get();
-        let old_owner = self.owner();
-        self.owner.store(None, Ordering::Relaxed);
-        // Make sure we release the lock before checking the assertions.
-        // We protect logging by a re-entrant lock, so we don't want
-        // to do any incidental logging while we the lock is held.
-        drop(guard_ptr.take());
-        // Now we have released the lock, it's okay to use logging.
-        assert_eq!(old_owner, Some(ThreadId::current()));
-    }
-    #[allow(unsafe_code)]
-    pub fn lock(&self) -> LockResult<()> {
-        let (guard, result) = match self.mutex.lock() {
-            Ok(guard) => (guard, Ok(())),
-            Err(err) => (err.into_inner(), Err(PoisonError::new(()))),
-        };
-        unsafe {
-            self.set_guard_and_owner(guard);
-        }
-        result
-    }
-    #[allow(unsafe_code)]
-    pub fn try_lock(&self) -> TryLockResult<()> {
-        let (guard, result) = match self.mutex.try_lock() {
-            Ok(guard) => (guard, Ok(())),
-            Err(TryLockError::WouldBlock) => return Err(TryLockError::WouldBlock),
-            Err(TryLockError::Poisoned(err)) => (
-                err.into_inner(),
-                Err(TryLockError::Poisoned(PoisonError::new(()))),
-            ),
-        };
-        unsafe {
-            self.set_guard_and_owner(guard);
-        }
-        result
-    }
-    #[allow(unsafe_code)]
-    pub fn unlock(&self) {
-        unsafe {
-            self.unset_guard_and_owner();
-        }
-    }
-    pub fn owner(&self) -> Option<ThreadId> {
-        self.owner.load(Ordering::Relaxed)
-    }
-}
-
-#[allow(unsafe_code)]
-unsafe impl Send for HandOverHandMutex {}
-
-/// A type for re-entrant mutexes.
-///
-/// It provides the same interface as <https://github.com/rust-lang/rust/blob/5edaa7eefd76d4996dcf85dfc1c1a3f737087257/src/libstd/sys_common/remutex.rs>
-
-pub struct ReentrantMutex<T> {
-    mutex: HandOverHandMutex,
-    count: Cell<usize>,
-    data: T,
-}
-
-#[allow(unsafe_code)]
-unsafe impl<T> Sync for ReentrantMutex<T> where T: Send {}
-
-impl<T> ReentrantMutex<T> {
-    pub fn new(data: T) -> ReentrantMutex<T> {
-        trace!("{:?} Creating new lock.", ThreadId::current());
-        ReentrantMutex {
-            mutex: HandOverHandMutex::default(),
-            count: Cell::new(0),
-            data,
-        }
-    }
-
-    pub fn lock(&self) -> LockResult<ReentrantMutexGuard<T>> {
-        trace!("{:?} Locking.", ThreadId::current());
-        if self.mutex.owner() != Some(ThreadId::current()) {
-            trace!("{:?} Becoming owner.", ThreadId::current());
-            if self.mutex.lock().is_err() {
-                trace!("{:?} Poison!", ThreadId::current());
-                return Err(PoisonError::new(self.mk_guard()));
-            }
-            trace!("{:?} Became owner.", ThreadId::current());
-        }
-        Ok(self.mk_guard())
-    }
-
-    pub fn try_lock(&self) -> TryLockResult<ReentrantMutexGuard<T>> {
-        trace!("{:?} Try locking.", ThreadId::current());
-        if self.mutex.owner() != Some(ThreadId::current()) {
-            trace!("{:?} Becoming owner?", ThreadId::current());
-            if let Err(err) = self.mutex.try_lock() {
-                match err {
-                    TryLockError::WouldBlock => {
-                        trace!("{:?} Would block.", ThreadId::current());
-                        return Err(TryLockError::WouldBlock);
-                    },
-                    TryLockError::Poisoned(_) => {
-                        trace!("{:?} Poison!", ThreadId::current());
-                        return Err(TryLockError::Poisoned(PoisonError::new(self.mk_guard())));
-                    },
-                }
-            }
-            trace!("{:?} Became owner.", ThreadId::current());
-        }
-        Ok(self.mk_guard())
-    }
-
-    fn unlock(&self) {
-        trace!("{:?} Unlocking.", ThreadId::current());
-        let count = self
-            .count
-            .get()
-            .checked_sub(1)
-            .expect("Underflowed lock count.");
-        trace!("{:?} Decrementing count to {}.", ThreadId::current(), count);
-        self.count.set(count);
-        if count == 0 {
-            trace!("{:?} Releasing mutex.", ThreadId::current());
-            self.mutex.unlock();
-        }
-    }
-
-    fn mk_guard(&self) -> ReentrantMutexGuard<T> {
-        let count = self
-            .count
-            .get()
-            .checked_add(1)
-            .expect("Overflowed lock count.");
-        trace!("{:?} Incrementing count to {}.", ThreadId::current(), count);
-        self.count.set(count);
-        ReentrantMutexGuard { mutex: self }
-    }
-}
-
-#[must_use]
-pub struct ReentrantMutexGuard<'a, T>
-where
-    T: 'static,
-{
-    mutex: &'a ReentrantMutex<T>,
-}
-
-impl<'a, T> Drop for ReentrantMutexGuard<'a, T> {
-    #[allow(unsafe_code)]
-    fn drop(&mut self) {
-        self.mutex.unlock()
-    }
-}
-
-impl<'a, T> Deref for ReentrantMutexGuard<'a, T> {
-    type Target = T;
-    fn deref(&self) -> &T {
-        &self.mutex.data
-    }
-}

components/remutex/tests/smoke.rs

@@ -1,92 +0,0 @@
-// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
-// file at the top-level directory of this distribution and at
-// http://rust-lang.org/COPYRIGHT.
-//
-// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
-// option. This file may not be copied, modified, or distributed
-// except according to those terms.
-
-// These tests came from https://github.com/rust-lang/rust/blob/master/src/libstd/sys/common/remutex.rs
-
-use std::cell::RefCell;
-use std::sync::Arc;
-use std::thread;
-
-use servo_remutex::{ReentrantMutex, ReentrantMutexGuard};
-
-#[test]
-fn smoke() {
-    let m = ReentrantMutex::new(());
-    {
-        let a = m.lock().unwrap();
-        {
-            let b = m.lock().unwrap();
-            {
-                let c = m.lock().unwrap();
-                assert_eq!(*c, ());
-            }
-            assert_eq!(*b, ());
-        }
-        assert_eq!(*a, ());
-    }
-}
-
-#[test]
-fn is_mutex() {
-    let m = Arc::new(ReentrantMutex::new(RefCell::new(0)));
-    let m2 = m.clone();
-    let lock = m.lock().unwrap();
-    let child = thread::spawn(move || {
-        let lock = m2.lock().unwrap();
-        assert_eq!(*lock.borrow(), 4950);
-    });
-    for i in 0..100 {
-        let lock = m.lock().unwrap();
-        *lock.borrow_mut() += i;
-    }
-    drop(lock);
-    child.join().unwrap();
-}
-
-#[test]
-fn trylock_works() {
-    let m = Arc::new(ReentrantMutex::new(()));
-    let m2 = m.clone();
-    let _lock = m.try_lock().unwrap();
-    let _lock2 = m.try_lock().unwrap();
-    thread::spawn(move || {
-        let lock = m2.try_lock();
-        assert!(lock.is_err());
-    })
-    .join()
-    .unwrap();
-    let _lock3 = m.try_lock().unwrap();
-}
-
-pub struct Answer<'a>(pub ReentrantMutexGuard<'a, RefCell<u32>>);
-impl<'a> Drop for Answer<'a> {
-    fn drop(&mut self) {
-        *self.0.borrow_mut() = 42;
-    }
-}
-
-#[test]
-fn poison_works() {
-    let m = Arc::new(ReentrantMutex::new(RefCell::new(0)));
-    let mc = m.clone();
-    let result = thread::spawn(move || {
-        let lock = mc.lock().unwrap();
-        *lock.borrow_mut() = 1;
-        let lock2 = mc.lock().unwrap();
-        *lock.borrow_mut() = 2;
-        let _answer = Answer(lock2);
-        println!("Intentionally panicking.");
-        panic!("What the answer to my lifetimes dilemma is?");
-    })
-    .join();
-    assert!(result.is_err());
-    let r = m.lock().err().unwrap().into_inner();
-    assert_eq!(*r.borrow(), 42);
-}

python/servo/testing_commands.py

@@ -159,7 +159,6 @@ class MachCommands(CommandBase):
             "selectors",
             "script_traits",
             "servo_config",
-            "servo_remutex",
             "crown",
             "constellation",
             "style_config",