diff --git a/CHANGELOG.md b/CHANGELOG.md index 3231ae5..4ffb0e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,5 +12,4 @@ - Deallocate previous stack when loading executable - - The `PageAllocator` would split blocks and them put the other half back at the incorrect size, - leaking an enourmous amount of memory. + - Free allocated regions when destroying threads diff --git a/Kernel/Loader.cpp b/Kernel/Loader.cpp index fcf2c9b..cc72aab 100644 --- a/Kernel/Loader.cpp +++ b/Kernel/Loader.cpp @@ -207,9 +207,11 @@ namespace Kernel // We allocated a stack for this thread in the Scheduler, free it here VERIFY(Scheduler::the().active_thread().m_owned_ranges.size() == 1); - for (auto range : Scheduler::the().active_thread().m_owned_ranges.iter()) { - PageAllocator::the().deallocate(range); - } + Scheduler::the().active_thread().free_owned_ranges(); + + // FIXME: This is a bit ugly, RAII for the PageRanges? + // Take ownership of the regions of this executable + Scheduler::the().active_thread().m_owned_ranges.append(PageRange{ power_of_two(executable.m_writable_size), executable.m_writable_base }); dbgln("[hand_over_to_loaded_executable] Droping privileges"); diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index 0968c9f..8d339be 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -110,7 +110,7 @@ namespace Kernel thread.m_context = context; if (thread.m_die_at_next_opportunity) { - // FIXME: Free stack here + thread.free_owned_ranges(); } else { m_threads.enqueue(move(thread)); } @@ -152,6 +152,8 @@ namespace Kernel void Scheduler::terminate_active_thread() { + dbgln("[Scheduler::terminate_active_thread]"); + active_thread().m_die_at_next_opportunity = true; donate_my_remaining_cpu_slice(); } diff --git a/Kernel/Scheduler.hpp b/Kernel/Scheduler.hpp index aa516db..323a402 100644 --- a/Kernel/Scheduler.hpp +++ b/Kernel/Scheduler.hpp @@ -88,6 +88,17 @@ namespace Kernel { } + // FIXME: RAII + void free_owned_ranges() + { + dbgln("[Thread::free_owned_ranges] name={}", m_name); + + for (auto range : m_owned_ranges.iter()) { + PageAllocator::the().deallocate(range); + } + m_owned_ranges.clear(); + } + Vector m_owned_ranges; String m_name; diff --git a/README.md b/README.md index 611e55d..b3a0fee 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,9 @@ ### TODO -#### Next Version +#### Bugs - - Free allocated segments when destroying threads + - We sometimes crash in `Process::create` when computing the new region. + This appears to be a race condition so the Scheduler is involved? #### Future features @@ -16,10 +17,11 @@ #### Future tweaks (Kernel) - - Free resources when threads and process are destroyed + - Use RAII to manage `PageRange`s - - We were allocating the stack twice; is this still accurate with - `posix_spawn`? + - Group `PageRange`s together in `PageAllocator::deallocate`. + + - Provide a shortcut for `Scheduler::the().active_thread()` similar: `Process::current()`. - Lookup devices via `devno`