Merge #106

106: avoid deadlock when multiple cores are available r=jbreitbart a=stlankes

- PR should solve issues #105
- add integration test to show this behavior
- make sure that the task is always added to the list of blocked task => avoid "loosing" of tasks

Co-authored-by: Stefan Lankes <[email protected]>

.github/workflows/test.yml

@@ -126,6 +126,7 @@ jobs:
         run:
           cargo test --tests --no-fail-fast -Z build-std=core,alloc --target x86_64-unknown-hermit-kernel -- --bootloader_path=../loader/target/x86_64-unknown-hermit-loader/debug/rusty-loader
         continue-on-error: true
-
-
-
+      - name: (Experimental) - Integration Tests (smp)
+        run:
+          cargo test --tests --no-fail-fast -Z build-std=core,alloc --target x86_64-unknown-hermit-kernel -- --bootloader_path=../loader/target/x86_64-unknown-hermit-loader/debug/rusty-loader --num_cores 2
+        continue-on-error: true

Cargo.toml

@@ -48,6 +48,10 @@ acpi = []
 x86_64 = "0.11.0"
 float-cmp = "0.8.0"
 
+[dependencies.scopeguard]
+version = "1.1"
+default-features = false
+
 [dev-dependencies.num-traits]
 version = "0.2"
 default-features = false

src/lib.rs

@@ -61,6 +61,7 @@ extern crate num;
 #[macro_use]
 extern crate num_derive;
 extern crate num_traits;
+extern crate scopeguard;
 #[cfg(not(target_os = "hermit"))]
 #[macro_use]
 extern crate std;

src/scheduler/mod.rs

@@ -369,9 +369,7 @@ impl PerCoreScheduler {
 	/// Triggers the scheduler to reschedule the tasks.
 	/// Interrupt flag will be cleared during the reschedule
 	pub fn reschedule(&mut self) {
-		let irq = irq::nested_disable();
-		self.scheduler();
-		irq::nested_enable(irq);
+		irqsave(|| self.scheduler());
 	}
 
 	/// Only the idle task should call this function to

src/scheduler/task.rs

@@ -479,6 +479,12 @@ struct BlockedTask {
 	wakeup_time: Option<u64>,
 }
 
+impl BlockedTask {
+	pub fn new(task: Rc<RefCell<Task>>, wakeup_time: Option<u64>) -> Self {
+		Self { task, wakeup_time }
+	}
+}
+
 pub struct BlockedTaskQueue {
 	list: LinkedList<BlockedTask>,
 }
@@ -520,6 +526,7 @@ impl BlockedTaskQueue {
 	}
 
 	/// Blocks the given task for `wakeup_time` ticks, or indefinitely if None is given.
+	#[allow(unused_assignments)]
 	pub fn add(&mut self, task: Rc<RefCell<Task>>, wakeup_time: Option<u64>) {
 		{
 			// Set the task status to Blocked.
@@ -535,30 +542,34 @@ impl BlockedTaskQueue {
 			borrowed.status = TaskStatus::TaskBlocked;
 		}
 
-		let new_node = BlockedTask { task, wakeup_time };
+		let new_node = BlockedTask::new(task, wakeup_time);
 
 		// Shall the task automatically be woken up after a certain time?
 		if let Some(wt) = wakeup_time {
 			let mut first_task = true;
 			let mut cursor = self.list.cursor_front_mut();
+			let mut _guard = scopeguard::guard(first_task, |first_task| {
+				// If the task is the new first task in the list, update the one-shot timer
+				// to fire when this task shall be woken up.
+				if first_task {
+					arch::set_oneshot_timer(wakeup_time);
+				}
+			});
 
 			while let Some(node) = cursor.current() {
 				let node_wakeup_time = node.wakeup_time;
 				if node_wakeup_time.is_none() || wt < node_wakeup_time.unwrap() {
 					cursor.insert_before(new_node);
 
-					// If this is the new first task in the list, update the One-Shot Timer
-					// to fire when this task shall be woken up.
-					if first_task {
-						arch::set_oneshot_timer(wakeup_time);
-					}
-
 					return;
 				}
 
 				first_task = false;
 				cursor.move_next();
 			}
+
+			// No, then just insert it at the end of the list.
+			self.list.push_back(new_node);
 		} else {
 			// No, then just insert it at the end of the list.
 			self.list.push_back(new_node);

tests/thread.rs

@@ -0,0 +1,47 @@
+#![feature(test)]
+#![no_std]
+#![no_main]
+#![test_runner(common::test_case_runner)]
+#![feature(custom_test_frameworks)]
+#![reexport_test_harness_main = "test_main"]
+
+extern crate hermit;
+
+#[macro_use]
+use common::*;
+mod common;
+
+#[macro_use]
+use alloc::vec;
+use hermit::{sys_join, sys_spawn2, sys_usleep, USER_STACK_SIZE};
+
+const NORMAL_PRIO: u8 = 2;
+
+extern "C" fn thread_func(i: usize) {
+	println!("this is thread number {}", i);
+	sys_usleep(2000000);
+	println!("---------------THREAD DONE!---------- {}", i);
+}
+
+#[test_case]
+pub fn thread_test() {
+	let mut children = vec![];
+
+	let threadnum = 5;
+	for i in 0..threadnum {
+		println!("SPAWNING THREAD {}", i);
+		let id = sys_spawn2(thread_func, i, NORMAL_PRIO, USER_STACK_SIZE, -1);
+		children.push(id);
+	}
+	println!("SPAWNED THREADS");
+
+	for child in children {
+		sys_join(child);
+	}
+}
+
+#[no_mangle]
+extern "C" fn runtime_entry(_argc: i32, _argv: *const *const u8, _env: *const *const u8) -> ! {
+	test_main();
+	common::exit(false)
+}