Skip to content

Commit 63a255b

Browse files
committed
Replace is_sync specialization trick with MaybeSync trait bound.
The `is_sync::<T>()` 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.
1 parent 151adc0 commit 63a255b

File tree

10 files changed

+57
-129
lines changed

10 files changed

+57
-129
lines changed

src/conversion.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::string::{BorrowedBytes, BorrowedStr, LuaString};
1616
use crate::table::Table;
1717
use crate::thread::Thread;
1818
use crate::traits::{FromLua, IntoLua, ShortTypeName as _};
19-
use crate::types::{Either, LightUserData, MaybeSend, RegistryKey};
19+
use crate::types::{Either, LightUserData, MaybeSend, MaybeSync, RegistryKey};
2020
use crate::userdata::{AnyUserData, UserData};
2121
use crate::value::{Nil, Value};
2222

@@ -294,7 +294,7 @@ impl FromLua for AnyUserData {
294294
}
295295
}
296296

297-
impl<T: UserData + MaybeSend + 'static> IntoLua for T {
297+
impl<T: UserData + MaybeSend + MaybeSync + 'static> IntoLua for T {
298298
#[inline]
299299
fn into_lua(self, lua: &Lua) -> Result<Value> {
300300
Ok(Value::UserData(lua.create_userdata(self)?))

src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ pub use crate::traits::{
113113
FromLua, FromLuaMulti, IntoLua, IntoLuaMulti, LuaNativeFn, LuaNativeFnMut, ObjectLike,
114114
};
115115
pub use crate::types::{
116-
AppDataRef, AppDataRefMut, Either, Integer, LightUserData, MaybeSend, Number, RegistryKey, VmState,
116+
AppDataRef, AppDataRefMut, Either, Integer, LightUserData, MaybeSend, MaybeSync, Number, RegistryKey,
117+
VmState,
117118
};
118119
pub use crate::userdata::{
119120
AnyUserData, MetaMethod, UserData, UserDataFields, UserDataMetatable, UserDataMethods, UserDataRef,

src/state.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ use crate::table::Table;
2020
use crate::thread::Thread;
2121
use crate::traits::{FromLua, FromLuaMulti, IntoLua, IntoLuaMulti};
2222
use crate::types::{
23-
AppDataRef, AppDataRefMut, ArcReentrantMutexGuard, Integer, LuaType, MaybeSend, Number, ReentrantMutex,
24-
ReentrantMutexGuard, RegistryKey, VmState, XRc, XWeak,
23+
AppDataRef, AppDataRefMut, ArcReentrantMutexGuard, Integer, LuaType, MaybeSend, MaybeSync, Number,
24+
ReentrantMutex, ReentrantMutexGuard, RegistryKey, VmState, XRc, XWeak,
2525
};
2626
use crate::userdata::{AnyUserData, UserData, UserDataProxy, UserDataRegistry, UserDataStorage};
2727
use crate::util::{StackGuard, assert_stack, check_stack, protect_lua_closure, push_string, rawset_field};
@@ -1492,7 +1492,7 @@ impl Lua {
14921492
#[inline]
14931493
pub fn create_userdata<T>(&self, data: T) -> Result<AnyUserData>
14941494
where
1495-
T: UserData + MaybeSend + 'static,
1495+
T: UserData + MaybeSend + MaybeSync + 'static,
14961496
{
14971497
unsafe { self.lock().make_userdata(UserDataStorage::new(data)) }
14981498
}
@@ -1503,7 +1503,7 @@ impl Lua {
15031503
#[inline]
15041504
pub fn create_ser_userdata<T>(&self, data: T) -> Result<AnyUserData>
15051505
where
1506-
T: UserData + Serialize + MaybeSend + 'static,
1506+
T: UserData + Serialize + MaybeSend + MaybeSync + 'static,
15071507
{
15081508
unsafe { self.lock().make_userdata(UserDataStorage::new_ser(data)) }
15091509
}
@@ -1518,7 +1518,7 @@ impl Lua {
15181518
#[inline]
15191519
pub fn create_any_userdata<T>(&self, data: T) -> Result<AnyUserData>
15201520
where
1521-
T: MaybeSend + 'static,
1521+
T: MaybeSend + MaybeSync + 'static,
15221522
{
15231523
unsafe { self.lock().make_any_userdata(UserDataStorage::new(data)) }
15241524
}
@@ -1531,7 +1531,7 @@ impl Lua {
15311531
#[inline]
15321532
pub fn create_ser_any_userdata<T>(&self, data: T) -> Result<AnyUserData>
15331533
where
1534-
T: Serialize + MaybeSend + 'static,
1534+
T: Serialize + MaybeSend + MaybeSync + 'static,
15351535
{
15361536
unsafe { (self.lock()).make_any_userdata(UserDataStorage::new_ser(data)) }
15371537
}

src/types.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,18 @@ pub trait MaybeSend {}
128128
#[cfg(not(feature = "send"))]
129129
impl<T> MaybeSend for T {}
130130

131+
/// A trait that adds `Sync` requirement if `send` feature is enabled.
132+
#[cfg(feature = "send")]
133+
pub trait MaybeSync: Sync {}
134+
#[cfg(feature = "send")]
135+
impl<T: Sync> MaybeSync for T {}
136+
137+
/// A trait that adds `Sync` requirement if `send` feature is enabled.
138+
#[cfg(not(feature = "send"))]
139+
pub trait MaybeSync {}
140+
#[cfg(not(feature = "send"))]
141+
impl<T> MaybeSync for T {}
142+
131143
pub(crate) struct DestructedUserdata;
132144

133145
pub(crate) trait LuaType {

src/userdata.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::state::Lua;
1010
use crate::string::LuaString;
1111
use crate::table::{Table, TablePairs};
1212
use crate::traits::{FromLua, FromLuaMulti, IntoLua, IntoLuaMulti};
13-
use crate::types::{MaybeSend, ValueRef};
13+
use crate::types::{MaybeSend, MaybeSync, ValueRef};
1414
use crate::util::{StackGuard, check_stack, get_userdata, push_string, short_type_name, take_userdata};
1515
use crate::value::Value;
1616

@@ -1216,7 +1216,7 @@ impl AnyUserData {
12161216
/// Wraps any Rust type, returning an opaque type that implements [`IntoLua`] trait.
12171217
///
12181218
/// This function uses [`Lua::create_any_userdata`] under the hood.
1219-
pub fn wrap<T: MaybeSend + 'static>(data: T) -> impl IntoLua {
1219+
pub fn wrap<T: MaybeSend + MaybeSync + 'static>(data: T) -> impl IntoLua {
12201220
WrappedUserdata(move |lua| lua.create_any_userdata(data))
12211221
}
12221222

@@ -1226,7 +1226,7 @@ impl AnyUserData {
12261226
/// This function uses [`Lua::create_ser_any_userdata`] under the hood.
12271227
#[cfg(feature = "serde")]
12281228
#[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
1229-
pub fn wrap_ser<T: Serialize + MaybeSend + 'static>(data: T) -> impl IntoLua {
1229+
pub fn wrap_ser<T: Serialize + MaybeSend + MaybeSync + 'static>(data: T) -> impl IntoLua {
12301230
WrappedUserdata(move |lua| lua.create_ser_any_userdata(data))
12311231
}
12321232
}

src/userdata/cell.rs

Lines changed: 18 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub(crate) enum UserDataStorage<T> {
2525
pub(crate) enum UserDataVariant<T> {
2626
Default(XRc<UserDataCell<T>>),
2727
#[cfg(feature = "serde")]
28-
Serializable(XRc<UserDataCell<Box<DynSerialize>>>, bool), // bool is `is_sync`
28+
Serializable(XRc<UserDataCell<Box<DynSerialize>>>),
2929
}
3030

3131
impl<T> Clone for UserDataVariant<T> {
@@ -34,18 +34,20 @@ impl<T> Clone for UserDataVariant<T> {
3434
match self {
3535
Self::Default(inner) => Self::Default(XRc::clone(inner)),
3636
#[cfg(feature = "serde")]
37-
Self::Serializable(inner, is_sync) => Self::Serializable(XRc::clone(inner), *is_sync),
37+
Self::Serializable(inner) => Self::Serializable(XRc::clone(inner)),
3838
}
3939
}
4040
}
4141

4242
impl<T> UserDataVariant<T> {
4343
#[inline(always)]
4444
pub(super) fn try_borrow_scoped<R>(&self, f: impl FnOnce(&T) -> R) -> Result<R> {
45-
// We don't need to check for `T: Sync` because when this method is used (internally),
46-
// Lua mutex is already locked.
47-
// If non-`Sync` userdata is already borrowed by another thread (via `UserDataRef`), it will be
48-
// exclusively locked.
45+
// Shared (read) lock is always correct for in-place borrows:
46+
// - this method is called internally while the Lua mutex is held, ensuring exclusive Lua-level
47+
// access per call frame
48+
// - with `send` feature, all owned userdata satisfies `T: Sync`, so simultaneous shared references
49+
// from multiple threads are sound
50+
// - without `send` feature, single-threaded execution makes shared lock safe for any `T`
4951
let _guard = (self.raw_lock().try_lock_shared_guarded()).map_err(|_| Error::UserDataBorrowError)?;
5052
Ok(f(unsafe { &*self.as_ptr() }))
5153
}
@@ -80,7 +82,7 @@ impl<T> UserDataVariant<T> {
8082
Ok(match self {
8183
Self::Default(inner) => XRc::into_inner(inner).unwrap().value.into_inner(),
8284
#[cfg(feature = "serde")]
83-
Self::Serializable(inner, _) => unsafe {
85+
Self::Serializable(inner) => unsafe {
8486
let raw = Box::into_raw(XRc::into_inner(inner).unwrap().value.into_inner());
8587
*Box::from_raw(raw as *mut T)
8688
},
@@ -92,7 +94,7 @@ impl<T> UserDataVariant<T> {
9294
match self {
9395
Self::Default(inner) => XRc::strong_count(inner),
9496
#[cfg(feature = "serde")]
95-
Self::Serializable(inner, _) => XRc::strong_count(inner),
97+
Self::Serializable(inner) => XRc::strong_count(inner),
9698
}
9799
}
98100

@@ -101,7 +103,7 @@ impl<T> UserDataVariant<T> {
101103
match self {
102104
Self::Default(inner) => &inner.raw_lock,
103105
#[cfg(feature = "serde")]
104-
Self::Serializable(inner, _) => &inner.raw_lock,
106+
Self::Serializable(inner) => &inner.raw_lock,
105107
}
106108
}
107109

@@ -110,7 +112,7 @@ impl<T> UserDataVariant<T> {
110112
match self {
111113
Self::Default(inner) => inner.value.get(),
112114
#[cfg(feature = "serde")]
113-
Self::Serializable(inner, _) => unsafe { &mut **(inner.value.get() as *mut Box<T>) },
115+
Self::Serializable(inner) => unsafe { &mut **(inner.value.get() as *mut Box<T>) },
114116
}
115117
}
116118
}
@@ -119,24 +121,10 @@ impl<T> UserDataVariant<T> {
119121
impl Serialize for UserDataStorage<()> {
120122
fn serialize<S: Serializer>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> {
121123
match self {
122-
Self::Owned(variant @ UserDataVariant::Serializable(inner, is_sync)) => unsafe {
123-
#[cfg(feature = "send")]
124-
if *is_sync {
125-
let _guard = (variant.raw_lock().try_lock_shared_guarded())
126-
.map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?;
127-
(*inner.value.get()).serialize(serializer)
128-
} else {
129-
let _guard = (variant.raw_lock().try_lock_exclusive_guarded())
130-
.map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?;
131-
(*inner.value.get()).serialize(serializer)
132-
}
133-
#[cfg(not(feature = "send"))]
134-
{
135-
let _ = is_sync;
136-
let _guard = (variant.raw_lock().try_lock_shared_guarded())
137-
.map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?;
138-
(*inner.value.get()).serialize(serializer)
139-
}
124+
Self::Owned(variant @ UserDataVariant::Serializable(inner)) => unsafe {
125+
let _guard = (variant.raw_lock().try_lock_shared_guarded())
126+
.map_err(|_| serde::ser::Error::custom(Error::UserDataBorrowError))?;
127+
(*inner.value.get()).serialize(serializer)
140128
},
141129
_ => Err(serde::ser::Error::custom("cannot serialize <userdata>")),
142130
}
@@ -201,11 +189,10 @@ impl<T: 'static> UserDataStorage<T> {
201189
#[inline(always)]
202190
pub(crate) fn new_ser(data: T) -> Self
203191
where
204-
T: Serialize + crate::types::MaybeSend,
192+
T: Serialize + crate::types::MaybeSend + crate::types::MaybeSync,
205193
{
206194
let data = Box::new(data) as Box<DynSerialize>;
207-
let is_sync = super::util::is_sync::<T>();
208-
let variant = UserDataVariant::Serializable(XRc::new(UserDataCell::new(data)), is_sync);
195+
let variant = UserDataVariant::Serializable(XRc::new(UserDataCell::new(data)));
209196
Self::Owned(variant)
210197
}
211198

src/userdata/ref.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use crate::value::Value;
1212

1313
use super::cell::{UserDataStorage, UserDataVariant};
1414
use super::lock::{LockGuard, RawLock, UserDataLock};
15-
use super::util::is_sync;
1615

1716
#[cfg(feature = "userdata-wrappers")]
1817
use {
@@ -63,11 +62,10 @@ impl<T> TryFrom<UserDataVariant<T>> for UserDataRef<T> {
6362

6463
#[inline]
6564
fn try_from(variant: UserDataVariant<T>) -> Result<Self> {
66-
let guard = if cfg!(not(feature = "send")) || is_sync::<T>() {
67-
variant.raw_lock().try_lock_shared_guarded()
68-
} else {
69-
variant.raw_lock().try_lock_exclusive_guarded()
70-
};
65+
// Shared (read) lock is always correct:
66+
// - with `send` feature, `T: Sync` is guaranteed by the `MaybeSync` bound on userdata creation
67+
// - without `send` feature, single-threaded access makes shared lock safe for any `T`
68+
let guard = variant.raw_lock().try_lock_shared_guarded();
7169
let guard = guard.map_err(|_| Error::UserDataBorrowError)?;
7270
let guard = unsafe { mem::transmute::<LockGuard<_>, LockGuard<'static, _>>(guard) };
7371
Ok(UserDataRef::from_parts(UserDataRefInner::Default(variant), guard))

src/userdata/registry.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,12 @@ macro_rules! lua_userdata_impl {
654654
// A special proxy object for UserData
655655
pub(crate) struct UserDataProxy<T>(pub(crate) PhantomData<T>);
656656

657+
// `UserDataProxy` holds no real `T` value, only a type marker, so it is always safe to send/share.
658+
#[cfg(feature = "send")]
659+
unsafe impl<T> Send for UserDataProxy<T> {}
660+
#[cfg(feature = "send")]
661+
unsafe impl<T> Sync for UserDataProxy<T> {}
662+
657663
lua_userdata_impl!(UserDataProxy<T>);
658664

659665
#[cfg(all(feature = "userdata-wrappers", not(feature = "send")))]

src/userdata/util.rs

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
use std::any::TypeId;
2-
use std::cell::Cell;
3-
use std::marker::PhantomData;
42
use std::os::raw::c_int;
53
use std::ptr;
64

@@ -11,35 +9,6 @@ use crate::error::{Error, Result};
119
use crate::types::CallbackPtr;
1210
use crate::util::{get_userdata, rawget_field, rawset_field, take_userdata};
1311

14-
// This is a trick to check if a type is `Sync` or not.
15-
// It uses leaked specialization feature from stdlib.
16-
struct IsSync<'a, T> {
17-
is_sync: &'a Cell<bool>,
18-
_marker: PhantomData<T>,
19-
}
20-
21-
impl<T> Clone for IsSync<'_, T> {
22-
fn clone(&self) -> Self {
23-
self.is_sync.set(false);
24-
IsSync {
25-
is_sync: self.is_sync,
26-
_marker: PhantomData,
27-
}
28-
}
29-
}
30-
31-
impl<T: Sync> Copy for IsSync<'_, T> {}
32-
33-
pub(crate) fn is_sync<T>() -> bool {
34-
let is_sync = Cell::new(true);
35-
let _ = [IsSync::<T> {
36-
is_sync: &is_sync,
37-
_marker: PhantomData,
38-
}]
39-
.clone();
40-
is_sync.get()
41-
}
42-
4312
// Userdata type hints, used to match types of wrapped userdata
4413
#[derive(Clone, Copy)]
4514
pub(crate) struct TypeIdHints {

tests/send.rs

Lines changed: 4 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,7 @@
11
#![cfg(feature = "send")]
22

3-
use std::cell::UnsafeCell;
4-
use std::marker::PhantomData;
5-
6-
use mlua::{AnyUserData, Error, Lua, ObjectLike, Result, UserData, UserDataMethods, UserDataRef};
7-
use static_assertions::{assert_impl_all, assert_not_impl_all};
8-
9-
#[test]
10-
fn test_userdata_multithread_access_send_only() -> Result<()> {
11-
let lua = Lua::new();
12-
13-
// This type is `Send` but not `Sync`.
14-
struct MyUserData(String, PhantomData<UnsafeCell<()>>);
15-
assert_impl_all!(MyUserData: Send);
16-
assert_not_impl_all!(MyUserData: Sync);
17-
18-
impl UserData for MyUserData {
19-
fn add_methods<M: UserDataMethods<Self>>(methods: &mut M) {
20-
methods.add_method("method", |lua, this, ()| {
21-
let ud = lua.globals().get::<AnyUserData>("ud")?;
22-
assert_eq!(ud.call_method::<String>("method2", ())?, "method2");
23-
Ok(this.0.clone())
24-
});
25-
26-
methods.add_method("method2", |_, _, ()| Ok("method2"));
27-
}
28-
}
29-
30-
lua.globals()
31-
.set("ud", MyUserData("hello".to_string(), PhantomData))?;
32-
33-
// We acquired the exclusive reference.
34-
let ud = lua.globals().get::<UserDataRef<MyUserData>>("ud")?;
35-
36-
std::thread::scope(|s| {
37-
s.spawn(|| {
38-
let res = lua.globals().get::<UserDataRef<MyUserData>>("ud");
39-
assert!(matches!(res, Err(Error::UserDataBorrowError)));
40-
});
41-
});
42-
43-
drop(ud);
44-
lua.load("ud:method()").exec().unwrap();
45-
46-
Ok(())
47-
}
3+
use mlua::{AnyUserData, Lua, ObjectLike, Result, UserData, UserDataMethods, UserDataRef};
4+
use static_assertions::assert_impl_all;
485

496
#[test]
507
fn test_userdata_multithread_access_sync() -> Result<()> {
@@ -74,13 +31,11 @@ fn test_userdata_multithread_access_sync() -> Result<()> {
7431
std::thread::scope(|s| {
7532
s.spawn(|| {
7633
// Getting another shared reference for `Sync` type is allowed.
77-
// FIXME: does not work due to https://github.com/rust-lang/rust/pull/135634
78-
// let _ = lua.globals().get::<UserDataRef<MyUserData>>("ud").unwrap();
34+
let _ = lua.globals().get::<UserDataRef<MyUserData>>("ud").unwrap();
7935
});
8036
});
8137

82-
// FIXME: does not work due to https://github.com/rust-lang/rust/pull/135634
83-
// lua.load("ud:method()").exec().unwrap();
38+
lua.load("ud:method()").exec().unwrap();
8439

8540
Ok(())
8641
}

0 commit comments

Comments
 (0)