From 63a255bbc918bf0925650313872608fdc838a57e Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Sat, 21 Feb 2026 15:05:55 +0000 Subject: [PATCH] Replace `is_sync` specialization trick with `MaybeSync` trait bound. The `is_sync::()` runtime check relied on implicit specialization via `Copy`/`Clone` array behavior, which has changed in Rust 1.86+. `UserDataRef` always taking an exclusive lock even for `Sync` userdata, preventing concurrent shared borrows. With the `send` feature flag enabled, userdata types must now be `Send + Sync`. This is a breaking change, `T: Send + !Sync` userdata types can be wrapped in a `Mutex` or used inside a `Scope` where this restriction is lifted. --- src/conversion.rs | 4 +-- src/lib.rs | 3 ++- src/state.rs | 12 ++++----- src/types.rs | 12 +++++++++ src/userdata.rs | 6 ++--- src/userdata/cell.rs | 49 ++++++++++++++----------------------- src/userdata/ref.rs | 10 +++----- src/userdata/registry.rs | 6 +++++ src/userdata/util.rs | 31 ----------------------- tests/send.rs | 53 +++------------------------------------- 10 files changed, 57 insertions(+), 129 deletions(-) diff --git a/src/conversion.rs b/src/conversion.rs index b74343d..8de5346 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -16,7 +16,7 @@ use crate::string::{BorrowedBytes, BorrowedStr, LuaString}; use crate::table::Table; use crate::thread::Thread; use crate::traits::{FromLua, IntoLua, ShortTypeName as _}; -use crate::types::{Either, LightUserData, MaybeSend, RegistryKey}; +use crate::types::{Either, LightUserData, MaybeSend, MaybeSync, RegistryKey}; use crate::userdata::{AnyUserData, UserData}; use crate::value::{Nil, Value}; @@ -294,7 +294,7 @@ impl FromLua for AnyUserData { } } -impl IntoLua for T { +impl IntoLua for T { #[inline] fn into_lua(self, lua: &Lua) -> Result { Ok(Value::UserData(lua.create_userdata(self)?)) diff --git a/src/lib.rs b/src/lib.rs index fa3bd1d..787baa3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -113,7 +113,8 @@ pub use crate::traits::{ FromLua, FromLuaMulti, IntoLua, IntoLuaMulti, LuaNativeFn, LuaNativeFnMut, ObjectLike, }; pub use crate::types::{ - AppDataRef, AppDataRefMut, Either, Integer, LightUserData, MaybeSend, Number, RegistryKey, VmState, + AppDataRef, AppDataRefMut, Either, Integer, LightUserData, MaybeSend, MaybeSync, Number, RegistryKey, + VmState, }; pub use crate::userdata::{ AnyUserData, MetaMethod, UserData, UserDataFields, UserDataMetatable, UserDataMethods, UserDataRef, diff --git a/src/state.rs b/src/state.rs index 9c364a2..543c006 100644 --- a/src/state.rs +++ b/src/state.rs @@ -20,8 +20,8 @@ use crate::table::Table; use crate::thread::Thread; use crate::traits::{FromLua, FromLuaMulti, IntoLua, IntoLuaMulti}; use crate::types::{ - AppDataRef, AppDataRefMut, ArcReentrantMutexGuard, Integer, LuaType, MaybeSend, Number, ReentrantMutex, - ReentrantMutexGuard, RegistryKey, VmState, XRc, XWeak, + AppDataRef, AppDataRefMut, ArcReentrantMutexGuard, Integer, LuaType, MaybeSend, MaybeSync, Number, + ReentrantMutex, ReentrantMutexGuard, RegistryKey, VmState, XRc, XWeak, }; use crate::userdata::{AnyUserData, UserData, UserDataProxy, UserDataRegistry, UserDataStorage}; use crate::util::{StackGuard, assert_stack, check_stack, protect_lua_closure, push_string, rawset_field}; @@ -1492,7 +1492,7 @@ impl Lua { #[inline] pub fn create_userdata(&self, data: T) -> Result where - T: UserData + MaybeSend + 'static, + T: UserData + MaybeSend + MaybeSync + 'static, { unsafe { self.lock().make_userdata(UserDataStorage::new(data)) } } @@ -1503,7 +1503,7 @@ impl Lua { #[inline] pub fn create_ser_userdata(&self, data: T) -> Result where - T: UserData + Serialize + MaybeSend + 'static, + T: UserData + Serialize + MaybeSend + MaybeSync + 'static, { unsafe { self.lock().make_userdata(UserDataStorage::new_ser(data)) } } @@ -1518,7 +1518,7 @@ impl Lua { #[inline] pub fn create_any_userdata(&self, data: T) -> Result where - T: MaybeSend + 'static, + T: MaybeSend + MaybeSync + 'static, { unsafe { self.lock().make_any_userdata(UserDataStorage::new(data)) } } @@ -1531,7 +1531,7 @@ impl Lua { #[inline] pub fn create_ser_any_userdata(&self, data: T) -> Result where - T: Serialize + MaybeSend + 'static, + T: Serialize + MaybeSend + MaybeSync + 'static, { unsafe { (self.lock()).make_any_userdata(UserDataStorage::new_ser(data)) } } diff --git a/src/types.rs b/src/types.rs index d05d35c..a6a9103 100644 --- a/src/types.rs +++ b/src/types.rs @@ -128,6 +128,18 @@ pub trait MaybeSend {} #[cfg(not(feature = "send"))] impl MaybeSend for T {} +/// A trait that adds `Sync` requirement if `send` feature is enabled. +#[cfg(feature = "send")] +pub trait MaybeSync: Sync {} +#[cfg(feature = "send")] +impl MaybeSync for T {} + +/// A trait that adds `Sync` requirement if `send` feature is enabled. +#[cfg(not(feature = "send"))] +pub trait MaybeSync {} +#[cfg(not(feature = "send"))] +impl MaybeSync for T {} + pub(crate) struct DestructedUserdata; pub(crate) trait LuaType { diff --git a/src/userdata.rs b/src/userdata.rs index 9a507d7..51ad517 100644 --- a/src/userdata.rs +++ b/src/userdata.rs @@ -10,7 +10,7 @@ use crate::state::Lua; use crate::string::LuaString; use crate::table::{Table, TablePairs}; use crate::traits::{FromLua, FromLuaMulti, IntoLua, IntoLuaMulti}; -use crate::types::{MaybeSend, ValueRef}; +use crate::types::{MaybeSend, MaybeSync, ValueRef}; use crate::util::{StackGuard, check_stack, get_userdata, push_string, short_type_name, take_userdata}; use crate::value::Value; @@ -1216,7 +1216,7 @@ impl AnyUserData { /// Wraps any Rust type, returning an opaque type that implements [`IntoLua`] trait. /// /// This function uses [`Lua::create_any_userdata`] under the hood. - pub fn wrap(data: T) -> impl IntoLua { + pub fn wrap(data: T) -> impl IntoLua { WrappedUserdata(move |lua| lua.create_any_userdata(data)) } @@ -1226,7 +1226,7 @@ impl AnyUserData { /// This function uses [`Lua::create_ser_any_userdata`] under the hood. #[cfg(feature = "serde")] #[cfg_attr(docsrs, doc(cfg(feature = "serde")))] - pub fn wrap_ser(data: T) -> impl IntoLua { + pub fn wrap_ser(data: T) -> impl IntoLua { WrappedUserdata(move |lua| lua.create_ser_any_userdata(data)) } } diff --git a/src/userdata/cell.rs b/src/userdata/cell.rs index 58f7246..5bd382f 100644 --- a/src/userdata/cell.rs +++ b/src/userdata/cell.rs @@ -25,7 +25,7 @@ pub(crate) enum UserDataStorage { pub(crate) enum UserDataVariant { Default(XRc>), #[cfg(feature = "serde")] - Serializable(XRc>>, bool), // bool is `is_sync` + Serializable(XRc>>), } impl Clone for UserDataVariant { @@ -34,7 +34,7 @@ impl Clone for UserDataVariant { match self { Self::Default(inner) => Self::Default(XRc::clone(inner)), #[cfg(feature = "serde")] - Self::Serializable(inner, is_sync) => Self::Serializable(XRc::clone(inner), *is_sync), + Self::Serializable(inner) => Self::Serializable(XRc::clone(inner)), } } } @@ -42,10 +42,12 @@ impl Clone for UserDataVariant { impl UserDataVariant { #[inline(always)] pub(super) fn try_borrow_scoped(&self, f: impl FnOnce(&T) -> R) -> Result { - // We don't need to check for `T: Sync` because when this method is used (internally), - // Lua mutex is already locked. - // If non-`Sync` userdata is already borrowed by another thread (via `UserDataRef`), it will be - // exclusively locked. + // Shared (read) lock is always correct for in-place borrows: + // - this method is called internally while the Lua mutex is held, ensuring exclusive Lua-level + // access per call frame + // - with `send` feature, all owned userdata satisfies `T: Sync`, so simultaneous shared references + // from multiple threads are sound + // - without `send` feature, single-threaded execution makes shared lock safe for any `T` let _guard = (self.raw_lock().try_lock_shared_guarded()).map_err(|_| Error::UserDataBorrowError)?; Ok(f(unsafe { &*self.as_ptr() })) } @@ -80,7 +82,7 @@ impl UserDataVariant { Ok(match self { Self::Default(inner) => XRc::into_inner(inner).unwrap().value.into_inner(), #[cfg(feature = "serde")] - Self::Serializable(inner, _) => unsafe { + Self::Serializable(inner) => unsafe { let raw = Box::into_raw(XRc::into_inner(inner).unwrap().value.into_inner()); *Box::from_raw(raw as *mut T) }, @@ -92,7 +94,7 @@ impl UserDataVariant { match self { Self::Default(inner) => XRc::strong_count(inner), #[cfg(feature = "serde")] - Self::Serializable(inner, _) => XRc::strong_count(inner), + Self::Serializable(inner) => XRc::strong_count(inner), } } @@ -101,7 +103,7 @@ impl UserDataVariant { match self { Self::Default(inner) => &inner.raw_lock, #[cfg(feature = "serde")] - Self::Serializable(inner, _) => &inner.raw_lock, + Self::Serializable(inner) => &inner.raw_lock, } } @@ -110,7 +112,7 @@ impl UserDataVariant { match self { Self::Default(inner) => inner.value.get(), #[cfg(feature = "serde")] - Self::Serializable(inner, _) => unsafe { &mut **(inner.value.get() as *mut Box) }, + Self::Serializable(inner) => unsafe { &mut **(inner.value.get() as *mut Box) }, } } } @@ -119,24 +121,10 @@ impl UserDataVariant { impl Serialize for UserDataStorage<()> { fn serialize(&self, serializer: S) -> std::result::Result { match self { - Self::Owned(variant @ UserDataVariant::Serializable(inner, is_sync)) => unsafe { - #[cfg(feature = "send")] - if *is_sync { - let _guard = (variant.raw_lock().try_lock_shared_guarded()) - .map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?; - (*inner.value.get()).serialize(serializer) - } else { - let _guard = (variant.raw_lock().try_lock_exclusive_guarded()) - .map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?; - (*inner.value.get()).serialize(serializer) - } - #[cfg(not(feature = "send"))] - { - let _ = is_sync; - let _guard = (variant.raw_lock().try_lock_shared_guarded()) - .map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?; - (*inner.value.get()).serialize(serializer) - } + Self::Owned(variant @ UserDataVariant::Serializable(inner)) => unsafe { + let _guard = (variant.raw_lock().try_lock_shared_guarded()) + .map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?; + (*inner.value.get()).serialize(serializer) }, _ => Err(serde::ser::Error::custom("cannot serialize ")), } @@ -201,11 +189,10 @@ impl UserDataStorage { #[inline(always)] pub(crate) fn new_ser(data: T) -> Self where - T: Serialize + crate::types::MaybeSend, + T: Serialize + crate::types::MaybeSend + crate::types::MaybeSync, { let data = Box::new(data) as Box; - let is_sync = super::util::is_sync::(); - let variant = UserDataVariant::Serializable(XRc::new(UserDataCell::new(data)), is_sync); + let variant = UserDataVariant::Serializable(XRc::new(UserDataCell::new(data))); Self::Owned(variant) } diff --git a/src/userdata/ref.rs b/src/userdata/ref.rs index 48f67c2..3cc59ef 100644 --- a/src/userdata/ref.rs +++ b/src/userdata/ref.rs @@ -12,7 +12,6 @@ use crate::value::Value; use super::cell::{UserDataStorage, UserDataVariant}; use super::lock::{LockGuard, RawLock, UserDataLock}; -use super::util::is_sync; #[cfg(feature = "userdata-wrappers")] use { @@ -63,11 +62,10 @@ impl TryFrom> for UserDataRef { #[inline] fn try_from(variant: UserDataVariant) -> Result { - let guard = if cfg!(not(feature = "send")) || is_sync::() { - variant.raw_lock().try_lock_shared_guarded() - } else { - variant.raw_lock().try_lock_exclusive_guarded() - }; + // Shared (read) lock is always correct: + // - with `send` feature, `T: Sync` is guaranteed by the `MaybeSync` bound on userdata creation + // - without `send` feature, single-threaded access makes shared lock safe for any `T` + let guard = variant.raw_lock().try_lock_shared_guarded(); let guard = guard.map_err(|_| Error::UserDataBorrowError)?; let guard = unsafe { mem::transmute::, LockGuard<'static, _>>(guard) }; Ok(UserDataRef::from_parts(UserDataRefInner::Default(variant), guard)) diff --git a/src/userdata/registry.rs b/src/userdata/registry.rs index e16bec6..c513814 100644 --- a/src/userdata/registry.rs +++ b/src/userdata/registry.rs @@ -654,6 +654,12 @@ macro_rules! lua_userdata_impl { // A special proxy object for UserData pub(crate) struct UserDataProxy(pub(crate) PhantomData); +// `UserDataProxy` holds no real `T` value, only a type marker, so it is always safe to send/share. +#[cfg(feature = "send")] +unsafe impl Send for UserDataProxy {} +#[cfg(feature = "send")] +unsafe impl Sync for UserDataProxy {} + lua_userdata_impl!(UserDataProxy); #[cfg(all(feature = "userdata-wrappers", not(feature = "send")))] diff --git a/src/userdata/util.rs b/src/userdata/util.rs index d42eaae..6c5f0f8 100644 --- a/src/userdata/util.rs +++ b/src/userdata/util.rs @@ -1,6 +1,4 @@ use std::any::TypeId; -use std::cell::Cell; -use std::marker::PhantomData; use std::os::raw::c_int; use std::ptr; @@ -11,35 +9,6 @@ use crate::error::{Error, Result}; use crate::types::CallbackPtr; use crate::util::{get_userdata, rawget_field, rawset_field, take_userdata}; -// This is a trick to check if a type is `Sync` or not. -// It uses leaked specialization feature from stdlib. -struct IsSync<'a, T> { - is_sync: &'a Cell, - _marker: PhantomData, -} - -impl Clone for IsSync<'_, T> { - fn clone(&self) -> Self { - self.is_sync.set(false); - IsSync { - is_sync: self.is_sync, - _marker: PhantomData, - } - } -} - -impl Copy for IsSync<'_, T> {} - -pub(crate) fn is_sync() -> bool { - let is_sync = Cell::new(true); - let _ = [IsSync:: { - is_sync: &is_sync, - _marker: PhantomData, - }] - .clone(); - is_sync.get() -} - // Userdata type hints, used to match types of wrapped userdata #[derive(Clone, Copy)] pub(crate) struct TypeIdHints { diff --git a/tests/send.rs b/tests/send.rs index f9803f5..2f10466 100644 --- a/tests/send.rs +++ b/tests/send.rs @@ -1,50 +1,7 @@ #![cfg(feature = "send")] -use std::cell::UnsafeCell; -use std::marker::PhantomData; - -use mlua::{AnyUserData, Error, Lua, ObjectLike, Result, UserData, UserDataMethods, UserDataRef}; -use static_assertions::{assert_impl_all, assert_not_impl_all}; - -#[test] -fn test_userdata_multithread_access_send_only() -> Result<()> { - let lua = Lua::new(); - - // This type is `Send` but not `Sync`. - struct MyUserData(String, PhantomData>); - assert_impl_all!(MyUserData: Send); - assert_not_impl_all!(MyUserData: Sync); - - impl UserData for MyUserData { - fn add_methods>(methods: &mut M) { - methods.add_method("method", |lua, this, ()| { - let ud = lua.globals().get::("ud")?; - assert_eq!(ud.call_method::("method2", ())?, "method2"); - Ok(this.0.clone()) - }); - - methods.add_method("method2", |_, _, ()| Ok("method2")); - } - } - - lua.globals() - .set("ud", MyUserData("hello".to_string(), PhantomData))?; - - // We acquired the exclusive reference. - let ud = lua.globals().get::>("ud")?; - - std::thread::scope(|s| { - s.spawn(|| { - let res = lua.globals().get::>("ud"); - assert!(matches!(res, Err(Error::UserDataBorrowError))); - }); - }); - - drop(ud); - lua.load("ud:method()").exec().unwrap(); - - Ok(()) -} +use mlua::{AnyUserData, Lua, ObjectLike, Result, UserData, UserDataMethods, UserDataRef}; +use static_assertions::assert_impl_all; #[test] fn test_userdata_multithread_access_sync() -> Result<()> { @@ -74,13 +31,11 @@ fn test_userdata_multithread_access_sync() -> Result<()> { std::thread::scope(|s| { s.spawn(|| { // Getting another shared reference for `Sync` type is allowed. - // FIXME: does not work due to https://github.com/rust-lang/rust/pull/135634 - // let _ = lua.globals().get::>("ud").unwrap(); + let _ = lua.globals().get::>("ud").unwrap(); }); }); - // FIXME: does not work due to https://github.com/rust-lang/rust/pull/135634 - // lua.load("ud:method()").exec().unwrap(); + lua.load("ud:method()").exec().unwrap(); Ok(()) }