Skip to content

Commit

Permalink
Kernel: Free allocated regions when destroying threads
Browse files Browse the repository at this point in the history
  • Loading branch information
asynts committed Jun 7, 2021
1 parent fce1289 commit 2194437
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 11 deletions.
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 5 additions & 3 deletions Kernel/Loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
4 changes: 3 additions & 1 deletion Kernel/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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();
}
Expand Down
11 changes: 11 additions & 0 deletions Kernel/Scheduler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<PageRange> m_owned_ranges;

String m_name;
Expand Down
12 changes: 7 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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`

Expand Down

0 comments on commit 2194437

Please sign in to comment.