Skip to content

Commit 23d4e25

Browse files
committed
Switch to Mutex from RwLock for userdata access in send mode.
Unfortunately RwLock allow access to the userdata from multiple threads without enforcing `Sync` marker.
1 parent 2857cb7 commit 23d4e25

File tree

3 files changed

+46
-17
lines changed

3 files changed

+46
-17
lines changed

src/userdata/cell.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ type DynSerialize = dyn erased_serde::Serialize + Send;
2727
pub(crate) enum UserDataVariant<T> {
2828
Default(XRc<UserDataCell<T>>),
2929
#[cfg(feature = "serialize")]
30-
Serializable(XRc<UserDataCell<ForceSync<Box<DynSerialize>>>>),
30+
Serializable(XRc<UserDataCell<Box<DynSerialize>>>),
3131
}
3232

3333
impl<T> Clone for UserDataVariant<T> {
@@ -82,7 +82,7 @@ impl<T> UserDataVariant<T> {
8282
Self::Default(inner) => XRc::into_inner(inner).unwrap().value.into_inner(),
8383
#[cfg(feature = "serialize")]
8484
Self::Serializable(inner) => unsafe {
85-
let raw = Box::into_raw(XRc::into_inner(inner).unwrap().value.into_inner().0);
85+
let raw = Box::into_raw(XRc::into_inner(inner).unwrap().value.into_inner());
8686
*Box::from_raw(raw as *mut T)
8787
},
8888
})
@@ -112,7 +112,6 @@ impl<T: Serialize + MaybeSend + 'static> UserDataVariant<T> {
112112
#[inline(always)]
113113
pub(crate) fn new_ser(data: T) -> Self {
114114
let data = Box::new(data) as Box<DynSerialize>;
115-
let data = ForceSync(data);
116115
Self::Serializable(XRc::new(UserDataCell::new(data)))
117116
}
118117
}
@@ -129,7 +128,7 @@ impl Serialize for UserDataVariant<()> {
129128
// No need to do this if the `send` feature is disabled.
130129
#[cfg(not(feature = "send"))]
131130
let _guard = self.try_borrow().map_err(serde::ser::Error::custom)?;
132-
(*inner.value.get()).0.serialize(serializer)
131+
(*inner.value.get()).serialize(serializer)
133132
},
134133
}
135134
}
@@ -142,7 +141,7 @@ pub(crate) struct UserDataCell<T> {
142141
}
143142

144143
unsafe impl<T: Send> Send for UserDataCell<T> {}
145-
unsafe impl<T: Send + Sync> Sync for UserDataCell<T> {}
144+
unsafe impl<T: Send> Sync for UserDataCell<T> {}
146145

147146
impl<T> UserDataCell<T> {
148147
#[inline(always)]
@@ -352,11 +351,6 @@ impl<'a, T> TryFrom<&'a UserDataVariant<T>> for UserDataBorrowMut<'a, T> {
352351
}
353352
}
354353

355-
#[repr(transparent)]
356-
pub(crate) struct ForceSync<T>(T);
357-
358-
unsafe impl<T: Send> Sync for ForceSync<T> {}
359-
360354
#[inline]
361355
fn try_value_to_userdata<T>(value: Value) -> Result<AnyUserData> {
362356
match value {

src/userdata/lock.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,32 +62,32 @@ mod lock_impl {
6262

6363
#[cfg(feature = "send")]
6464
mod lock_impl {
65-
use parking_lot::lock_api::RawRwLock;
65+
use parking_lot::lock_api::RawMutex;
6666

67-
pub(crate) type RawLock = parking_lot::RawRwLock;
67+
pub(crate) type RawLock = parking_lot::RawMutex;
6868

6969
impl super::UserDataLock for RawLock {
7070
#[allow(clippy::declare_interior_mutable_const)]
71-
const INIT: Self = <Self as parking_lot::lock_api::RawRwLock>::INIT;
71+
const INIT: Self = <Self as parking_lot::lock_api::RawMutex>::INIT;
7272

7373
#[inline(always)]
7474
fn try_lock_shared(&self) -> bool {
75-
RawRwLock::try_lock_shared(self)
75+
RawLock::try_lock(self)
7676
}
7777

7878
#[inline(always)]
7979
fn try_lock_exclusive(&self) -> bool {
80-
RawRwLock::try_lock_exclusive(self)
80+
RawLock::try_lock(self)
8181
}
8282

8383
#[inline(always)]
8484
unsafe fn unlock_shared(&self) {
85-
RawRwLock::unlock_shared(self)
85+
RawLock::unlock(self)
8686
}
8787

8888
#[inline(always)]
8989
unsafe fn unlock_exclusive(&self) {
90-
RawRwLock::unlock_exclusive(self)
90+
RawLock::unlock(self)
9191
}
9292
}
9393
}

tests/send.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#![cfg(feature = "send")]
2+
3+
use std::cell::UnsafeCell;
4+
use std::marker::PhantomData;
5+
use std::string::String as StdString;
6+
7+
use mlua::{AnyUserData, Error, Lua, Result, UserDataRef};
8+
use static_assertions::{assert_impl_all, assert_not_impl_all};
9+
10+
#[test]
11+
fn test_userdata_multithread_access() -> Result<()> {
12+
let lua = Lua::new();
13+
14+
// This type is `Send` but not `Sync`.
15+
struct MyUserData(#[allow(unused)] StdString, PhantomData<UnsafeCell<()>>);
16+
17+
assert_impl_all!(MyUserData: Send);
18+
assert_not_impl_all!(MyUserData: Sync);
19+
20+
lua.globals().set(
21+
"ud",
22+
AnyUserData::wrap(MyUserData("hello".to_string(), PhantomData)),
23+
)?;
24+
// We acquired the exclusive reference.
25+
let _ud1 = lua.globals().get::<UserDataRef<MyUserData>>("ud")?;
26+
27+
std::thread::scope(|s| {
28+
s.spawn(|| {
29+
let res = lua.globals().get::<UserDataRef<MyUserData>>("ud");
30+
assert!(matches!(res, Err(Error::UserDataBorrowError)));
31+
});
32+
});
33+
34+
Ok(())
35+
}

0 commit comments

Comments
 (0)