implement ExitThread

[LinusU]

No description provided.

[evmar]

This area is a little weird.

Right now CPUState::Exit is used to mark "the process overall wants to terminate" (see the implementation of exit_process), and so the various places that consider CPU state, like this one, treat any CPU going into the exit state as a "quit the process now" marker.

Meanwhile, new_cpu (the function used to start a new thread) doesn't consider any "free" CPUs (as would be freed up by ExitThread) for reuse, hmm.

I think with this modification it's possible for this loop to not find a CPU that is ready to run (if all the CPUs got into the exit state), which would mean the soonest.unwrap below would fail.

I think, having typed all that out, calling this state "Exit" is the problem because it muddles "exit process" with "exit thread". So I think what you should do instead is make a new CPUState, maybe ::Free, that a terminated thread goes into when it's no longer in use, and then use that in ExitThread, retrowin32_thread_main, and new_cpu.

[evmar]

BTW, if you run exe/rust/build.sh it builds a "threads.exe" program that can exercise some of this.
https://github.com/evmar/retrowin32/blob/main/exe/rust/README.md

I just added some build setup docs on how to build this:
https://github.com/evmar/retrowin32/blob/main/doc/build_setup.md

Not sure it's worth it for you, just FYI in case it helps

[LinusU]

I've added a new Free state, and was going to implement cpu reuse in new_cpu but not sure exactly how to proceed 🤔

diff --git a/x86/src/x86.rs b/x86/src/x86.rs
index 222969de..cfd8f386 100644
--- a/x86/src/x86.rs
+++ b/x86/src/x86.rs
@@ -69,6 +69,14 @@ impl CPU {
         self.state = CPUState::Error(msg);
     }
 
+    pub fn reset(&mut self) {
+        self.regs = Registers::default();
+        self.flags = Flags::empty();
+        self.fpu = FPU::default();
+        self.state = Default::default();
+        self.futures.clear();
+    }
+
     // /// Check whether reading a T from mem[addr] would cause OOB, and crash() if so.
     // fn check_oob<T>(&mut self, addr: u32) -> bool {
     //     if addr < NULL_POINTER_REGION_SIZE {
@@ -210,6 +218,13 @@ impl X86 {
     }
 
     pub fn new_cpu(&mut self) -> &mut CPU {
+        for i in 0..self.cpus.len() {
+            if self.cpus[i].state == CPUState::Free {
+                self.cpus[i].reset();
+                return &mut *self.cpus[i];
+            }
+        }
+
         self.cpus.push(Box::pin(CPU::new()));
         self.cpus.last_mut().unwrap()
     }

I started doing something like this, but I realized that I'm not too well versed in what parts should be reset. In particular, with the code I have, it seems like it might be better to just swap_remove the cpu instead of keeping it around as free? 🤔

[evmar]

Oh yeah, swap_remove makes sense to me! I wonder if there's some way you can swap_remove the current CPU such that we don't even need the ::Free state, but I think it's probably hard for a CPU to tear itself down when the code is deep within some call stack involving it...

[evmar]

FYI I dropped CPUStatus::Exit here c1735a7

[LinusU]

(rebased on latest main, not made any more progress)

[evmar]

This looks good, I merged it with another commit on top! We can refine the cleanup bits later, I don't imagine there are too many threads starting/exiting in the programs we care about yet. :)