Skip to content

Commit ec26d3d

Browse files
committed
Fix stack underflow in thread event callbacks
Introduce a new guard to reject resuming/resetting active thread from within it's own callback. Before it was possible to mutate the thread stack and making xmove to move more values than remain on the thread stack.
1 parent 406c18f commit ec26d3d

5 files changed

Lines changed: 186 additions & 82 deletions

File tree

src/state.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,9 @@ impl Lua {
889889
/// Only one callback can be registered at a time. Calling this again replaces the previous
890890
/// callback and its triggers.
891891
///
892+
/// If the callback returns an error, it's propagated out of the operation that triggered the
893+
/// event. For a [`ThreadEvent::Yield`], the yielded values are discarded.
894+
///
892895
/// # Example
893896
///
894897
/// Subscribe only to yield events:
@@ -949,17 +952,18 @@ impl Lua {
949952
}
950953

951954
let extra = ExtraData::get(child);
952-
if !(*extra).thread_triggers.on_create {
955+
if !(*extra).thread_triggers.on_create || !(*extra).thread_event_state.is_null() {
953956
return;
954957
}
955958
let callback = match &(*extra).thread_event_callback {
956-
Some(cb) if XRc::strong_count(cb) == 1 => cb.clone(),
959+
Some(cb) => cb.clone(),
957960
_ => return,
958961
};
959962
ffi::lua_pushthread(child);
960963
ffi::lua_xmove(child, (*extra).ref_thread, 1);
961964
let thread = Thread((*extra).raw_lua().pop_ref_thread(), child);
962965
callback_error_ext(parent, extra, false, move |extra, _| {
966+
let _guard = crate::thread::ThreadEventGuard::new((*extra).raw_lua(), child);
963967
callback((*extra).lua(), ThreadEvent::Create(thread))
964968
})
965969
}

src/state/extra.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ pub(crate) struct ExtraData {
8787
pub(super) interrupt_callback: Option<crate::types::InterruptCallback>,
8888
pub(super) thread_triggers: ThreadTriggers,
8989
pub(super) thread_event_callback: Option<ThreadEventCallback>,
90+
pub(super) thread_event_state: *mut ffi::lua_State,
9091

9192
#[cfg(feature = "luau")]
9293
pub(crate) running_gc: bool,
@@ -192,6 +193,7 @@ impl ExtraData {
192193
interrupt_callback: None,
193194
thread_triggers: ThreadTriggers::default(),
194195
thread_event_callback: None,
196+
thread_event_state: ptr::null_mut(),
195197
#[cfg(feature = "luau")]
196198
sandboxed: false,
197199
#[cfg(feature = "luau")]

src/state/raw.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -659,12 +659,10 @@ impl RawLua {
659659

660660
// Exec creation callback for non-Luau (Luau handles this via `userthread_proc`)
661661
#[cfg(not(feature = "luau"))]
662-
if self.thread_event_triggers().on_create {
662+
if self.thread_event_triggers().on_create && self.thread_event_state().is_null() {
663663
let extra = self.extra.get();
664-
if let Some(ref cb) = (*extra).thread_event_callback
665-
&& XRc::strong_count(cb) == 1
666-
{
667-
let cb = cb.clone();
664+
if let Some(cb) = (*extra).thread_event_callback.clone() {
665+
let _guard = crate::thread::ThreadEventGuard::new(self, thread_state);
668666
cb((*extra).lua(), crate::thread::ThreadEvent::Create(thread.clone()))?;
669667
}
670668
}
@@ -732,6 +730,16 @@ impl RawLua {
732730
(*self.extra.get()).thread_event_callback.clone()
733731
}
734732

733+
#[inline(always)]
734+
pub(crate) unsafe fn thread_event_state(&self) -> *mut ffi::lua_State {
735+
(*self.extra.get()).thread_event_state
736+
}
737+
738+
#[inline(always)]
739+
pub(crate) unsafe fn set_thread_event_state(&self, state: *mut ffi::lua_State) {
740+
(*self.extra.get()).thread_event_state = state;
741+
}
742+
735743
/// Pushes a primitive type value onto the Lua stack.
736744
pub(crate) unsafe fn push_primitive_type<T: LuaType>(&self) -> bool {
737745
match T::TYPE_ID {

0 commit comments

Comments
 (0)