From b9c5a523089061da30070861d2ce2b2277e921be Mon Sep 17 00:00:00 2001 From: Brian Misiak Date: Tue, 27 Feb 2024 12:27:46 -0800 Subject: [PATCH] safer --- src/lib.rs | 9 +++++---- src/scheduling.rs | 48 +++++++++++++++++++++++++++++------------------ src/timer.rs | 2 +- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a781932..287c70c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -52,7 +52,7 @@ impl PreciseTimers { next_trigger: Instant::now() + interval, repeat: if repeat { Every(interval) } else { DontRepeat }, }; - let key = insert_and_schedule_timer(timer, schedule); + let key = insert_and_schedule_timer(timer, schedule).map_err(|_| AmxError::MemoryAccess)?; // The timer's slot in Slab<> incresed by 1, so that 0 signifies an invalid timer in PAWN let timer_number = key .checked_add(1) @@ -123,15 +123,16 @@ impl SampPlugin for PreciseTimers { let now = Instant::now(); loop { - match trigger_next_due_and_then(now, |timer| timer.stack_callback()) { + match trigger_next_due_and_then(now, Timer::stack_callback_on_amx) { Ok(None) => break, - Ok(Some(callback)) => { + Ok(Some(Ok(callback))) => { // SAFETY: Must not hold any references to scheduling stores. if let Err(err) = unsafe { callback.execute() } { error!("Error while executing timer: {err}"); } } - Err(err) => error!("Error triggering next timer: {err}"), + Ok(Some(Err(stacking_err))) => error!("Failed to stack callback: {stacking_err}"), + Err(triggering_err) => error!("Error triggering next timer: {triggering_err}"), } } } diff --git a/src/scheduling.rs b/src/scheduling.rs index 9a4eb41..dd52b0c 100644 --- a/src/scheduling.rs +++ b/src/scheduling.rs @@ -29,7 +29,9 @@ pub(crate) enum TriggeringError { #[snafu(display("Unable to find timer in priority queue"))] TimerNotInQueue, #[snafu(display("Failed to get access to priority queue"))] - ReschedulingBorrow { source: BorrowMutError }, + QueueBorrowed { source: BorrowMutError }, + #[snafu(display("Inserting timer failed, unable to access store"))] + Inserting { source: BorrowMutError }, #[snafu(display("Popped timer is different from the expected due timer"))] Inconsistency, #[snafu(display("Timer was expected to be present in slab"))] @@ -40,10 +42,20 @@ pub(crate) enum TriggeringError { StackPush { source: AmxError }, } -pub(crate) fn insert_and_schedule_timer(timer: Timer, scheduling: Schedule) -> usize { - let key: usize = TIMERS.with_borrow_mut(|t| t.insert(timer)); - QUEUE.with_borrow_mut(|q| q.push(key, Reverse(scheduling))); - key +pub(crate) fn insert_and_schedule_timer( + timer: Timer, + scheduling: Schedule, +) -> Result { + let key: usize = TIMERS + .with(|t| t.try_borrow_mut().map(|mut t| t.insert(timer))) + .context(InsertingSnafu)?; + QUEUE + .with(|q| { + q.try_borrow_mut() + .map(|mut q| q.push(key, Reverse(scheduling))) + }) + .context(InsertingSnafu)?; + Ok(key) } pub(crate) fn delete_timer(timer_key: usize) -> Result, BorrowMutError> { @@ -57,6 +69,16 @@ pub(crate) fn delete_timer(timer_key: usize) -> Result, BorrowMutE })?)) } +pub(crate) fn reschedule_timer(key: usize, new_schedule: Schedule) -> Result<(), TriggeringError> { + QUEUE.with(|q| { + q.try_borrow_mut() + .context(QueueBorrowedSnafu)? + .change_priority(&key, Reverse(new_schedule)) + .map(|_| ()) + .context(TimerNotInQueueSnafu) + }) +} + pub(crate) fn remove_timers(predicate: impl Fn(&Timer) -> bool) { TIMERS.with_borrow_mut(|timers| { QUEUE.with_borrow_mut(|queue| { @@ -72,16 +94,6 @@ pub(crate) fn remove_timers(predicate: impl Fn(&Timer) -> bool) { }); } -pub(crate) fn reschedule_timer(key: usize, new_schedule: Schedule) -> Result<(), TriggeringError> { - QUEUE.with(|q| { - q.try_borrow_mut() - .context(ReschedulingBorrowSnafu)? - .change_priority(&key, Reverse(new_schedule)) - .map(|_| ()) - .context(TimerNotInQueueSnafu) - }) -} - /// 1. Reschedules (or deschedules) the timer /// 2. While holding the timer, gives it to the closure /// (which uses its data to push onto the amx stack) @@ -91,7 +103,7 @@ pub(crate) fn reschedule_timer(key: usize, new_schedule: Schedule) -> Result<(), #[inline] pub(crate) fn trigger_next_due_and_then( now: Instant, - prep: impl Fn(&Timer) -> Result, + timer_manipulator: impl Fn(&Timer) -> T, ) -> Result, TriggeringError> { QUEUE.with_borrow_mut(|q| { let Some((&key, &Reverse(scheduled))) = q.peek() else { @@ -107,14 +119,14 @@ pub(crate) fn trigger_next_due_and_then( }); TIMERS.with_borrow_mut(|t| { let timer = t.get_mut(key).context(ExpectedInSlabSnafu)?; - Ok(Some(prep(timer).context(StackPushSnafu)?)) + Ok(Some(timer_manipulator(timer))) }) } else { let (descheduled, _) = q.pop().context(TimerNotInQueueSnafu)?; ensure!(descheduled == key, InconsistencySnafu); let timer = TIMERS.with_borrow_mut(|t| t.remove(key)); - Ok(Some(prep(&timer).context(StackPushSnafu)?)) + Ok(Some(timer_manipulator(&timer))) } }) } diff --git a/src/timer.rs b/src/timer.rs index 7cdf014..edff6c4 100644 --- a/src/timer.rs +++ b/src/timer.rs @@ -19,7 +19,7 @@ impl Timer { self.amx_identifier == AmxIdent::from(amx.amx().as_ptr()) } - pub fn stack_callback(&self) -> Result { + pub fn stack_callback_on_amx(&self) -> Result { let amx: &'static Amx = samp::amx::get(self.amx_identifier).ok_or(AmxError::NotFound)?; self.passed_arguments .push_onto_amx_stack(amx, self.amx_callback_index)