Skip to content

Commit 792e4e7

Browse files
prabakaranklstpawelrutkaq
authored andcommitted
scheduler: Fix bug in scheduling safety task
When a task results in error, then it is scheduled on safety worker if enabled. Fixed a bug in wake/schedule of safety tasks which occurs when nested wakers/tasks are used.
1 parent 4d02b76 commit 792e4e7

File tree

6 files changed

+69
-76
lines changed

6 files changed

+69
-76
lines changed

src/kyron/src/scheduler/context.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,9 @@ pub(crate) struct WorkerContext {
318318
/// Helper flag to check if safety was enabled in runtime builder
319319
is_safety_enabled: bool,
320320

321+
/// Flag to check if current task resulted in safety error, then schedule on safety worker from wake function.
322+
is_task_safety_error: bool,
323+
321324
wakeup_time: Cell<Option<u64>>,
322325
}
323326

@@ -399,6 +402,7 @@ impl ContextBuilder {
399402
worker_id: Cell::new(self.worker_id.expect("Worker type must be set in context builder!")),
400403
handler: RefCell::new(Some(Rc::new(self.handle.expect("Handler type must be set in context builder!")))),
401404
is_safety_enabled: self.is_with_safety,
405+
is_task_safety_error: false,
402406
wakeup_time: Cell::new(None),
403407
drivers: Some(self.drivers),
404408
}
@@ -452,6 +456,32 @@ pub(crate) fn ctx_is_with_safety() -> bool {
452456
.unwrap_or_default()
453457
}
454458

459+
#[allow(dead_code)] // mock function is used in tests instead of this one
460+
///
461+
/// Sets task safety error flag
462+
///
463+
pub(crate) fn ctx_set_task_safety_error(is_error: bool) {
464+
CTX.try_with(|ctx| {
465+
ctx.borrow_mut().as_mut().expect("Called before CTX init?").is_task_safety_error = is_error;
466+
})
467+
.unwrap_or_default();
468+
}
469+
470+
#[allow(dead_code)] // mock function is used in tests instead of this one
471+
///
472+
/// Check if current task resulted in safety error and clear the flag.
473+
///
474+
pub(crate) fn ctx_check_task_safety_error_and_clear() -> bool {
475+
CTX.try_with(|ctx| {
476+
let mut binding = ctx.borrow_mut();
477+
let ctx = binding.as_mut().expect("Called before CTX init?");
478+
let val = ctx.is_task_safety_error;
479+
ctx.is_task_safety_error = false;
480+
val
481+
})
482+
.unwrap_or_default()
483+
}
484+
455485
pub(crate) fn ctx_set_wakeup_time(time: u64) {
456486
CTX.try_with(|ctx| ctx.borrow().as_ref().expect("Called before CTX init?").wakeup_time.set(Some(time)))
457487
.unwrap_or_default();

src/kyron/src/scheduler/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ pub(crate) mod execution_engine;
1717
pub(crate) mod join_handle;
1818
pub(crate) mod workers;
1919

20-
pub mod safety_waker;
2120
pub mod scheduler_mt;
2221
pub mod task;
2322
pub mod waker;

src/kyron/src/scheduler/safety_waker.rs

Lines changed: 0 additions & 68 deletions
This file was deleted.

src/kyron/src/scheduler/task/async_task.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@
1313

1414
use super::task_state::*;
1515
use crate::core::types::*;
16-
use crate::scheduler::safety_waker::create_safety_waker;
16+
#[cfg(not(any(test, feature = "runtime-api-mock")))]
17+
use crate::scheduler::context::ctx_set_task_safety_error;
1718
use crate::scheduler::scheduler_mt::SchedulerTrait;
19+
#[cfg(any(test, feature = "runtime-api-mock"))]
20+
use crate::testing::mock::ctx_set_task_safety_error;
1821
use ::core::future::Future;
1922
use ::core::mem;
2023
use ::core::ops::{Deref, DerefMut};
@@ -308,9 +311,8 @@ where
308311
self.handle_waker.with_mut(|ptr: *mut Option<Waker>| match unsafe { (*ptr).take() } {
309312
Some(v) => {
310313
if is_safety_err && self.is_with_safety {
311-
unsafe {
312-
create_safety_waker(v).wake();
313-
}
314+
ctx_set_task_safety_error(true);
315+
v.wake();
314316
} else {
315317
v.wake();
316318
}

src/kyron/src/scheduler/waker.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414
use kyron_foundation::prelude::FoundationAtomicPtr;
1515

1616
use super::task::async_task::*;
17+
#[cfg(not(any(test, feature = "runtime-api-mock")))]
18+
use crate::scheduler::context::ctx_check_task_safety_error_and_clear;
19+
#[cfg(any(test, feature = "runtime-api-mock"))]
20+
use crate::testing::mock::ctx_check_task_safety_error_and_clear;
1721
use core::task::{RawWaker, RawWakerVTable, Waker};
1822

1923
fn clone_waker(data: *const ()) -> RawWaker {
@@ -32,16 +36,23 @@ fn wake(data: *const ()) {
3236
let task_header_ptr = data as *const TaskHeader;
3337
let task_ref = unsafe { TaskRef::from_raw(task_header_ptr) };
3438

35-
task_ref.schedule();
36-
39+
if ctx_check_task_safety_error_and_clear() {
40+
task_ref.schedule_safety();
41+
} else {
42+
task_ref.schedule();
43+
}
3744
drop(task_ref); // wake uses move semantic, so we are owner of data now, so we need to cleanup
3845
}
3946

4047
fn wake_by_ref(data: *const ()) {
4148
let task_header_ptr = data as *const TaskHeader;
4249
let task_ref = unsafe { TaskRef::from_raw(task_header_ptr) };
4350

44-
task_ref.schedule();
51+
if ctx_check_task_safety_error_and_clear() {
52+
task_ref.schedule_safety();
53+
} else {
54+
task_ref.schedule();
55+
}
4556

4657
::core::mem::forget(task_ref); // don't touch refcount from our data since this is done by drop_waker
4758
}

src/kyron/src/testing/mock.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,25 @@ where
225225
}
226226
}
227227

228+
// Thread-local storage to track task safety errors
229+
thread_local! {
230+
static THREAD_CTX_SAFETY_ERROR_FLAG: RefCell<bool> = const { RefCell::new(false) };
231+
}
232+
233+
pub fn ctx_set_task_safety_error(is_error: bool) {
234+
THREAD_CTX_SAFETY_ERROR_FLAG.with(|flag| {
235+
*flag.borrow_mut() = is_error;
236+
});
237+
}
238+
239+
pub fn ctx_check_task_safety_error_and_clear() -> bool {
240+
THREAD_CTX_SAFETY_ERROR_FLAG.with(|flag| {
241+
let current = *flag.borrow();
242+
*flag.borrow_mut() = false;
243+
current
244+
})
245+
}
246+
228247
///
229248
/// Spawns a given `future` into runtime and let it execute on dedicated worker using `worker_id`.
230249
/// This function allocates a `future` dynamically using [`Box`]

0 commit comments

Comments
 (0)