From a5666328b703d12a6856470a7c1346860b654a2b Mon Sep 17 00:00:00 2001 From: Aryadev Chavali Date: Fri, 12 Dec 2025 04:29:32 +0000 Subject: [PATCH] Fixed the heap-use-after-free issue. Just standard multithreading stuff; access to the allocator while hot threads are running means stuff can change underneath us even /during/ a read. I've mutex locked state for stuff in the drawing domain which stops this issue. --- README.org | 6 +++++- src/main.cpp | 4 +++- src/state.cpp | 2 ++ src/state.hpp | 4 ++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/README.org b/README.org index c30e0ef..268e5f8 100644 --- a/README.org +++ b/README.org @@ -30,10 +30,14 @@ on the number line. We already have the latest bound nodes so we're part-way through the tree. Just keep going down what we have so far surely? Even better, don't use nodes _at all_. Run with an index! -** TODO Fix weird issue at past 100K nodes +** DONE Fix weird issue at past 100K nodes std::vector seems to crap itself past 100K nodes - we keep getting heap-use-after-free issues when trying to access the allocator nodes at that point. Seemingly random. What's going on? + +Solution: _everytime_ we want to access the allocator for nontrivial +(like memory reads) we need to lock the mutex. No two ways about it. +All draw functions were causing the issue. ** DONE Prettify code base It's a big blob of code currently in the graphics portion. Not very pretty but it gets the job done. Try modularisation. diff --git a/src/main.cpp b/src/main.cpp index 91aae32..233d1dd 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -52,7 +52,7 @@ constexpr u64 clamp_to_width(const DrawState &ds, f64 val) (val - ds.bounds.lower_val); } -void draw_tree(const DrawState &ds, const State &state) +void draw_tree(DrawState &ds, State &state) { // Number line DrawLine(0, HEIGHT / 2, WIDTH, HEIGHT / 2, WHITE); @@ -63,6 +63,7 @@ void draw_tree(const DrawState &ds, const State &state) DrawLine(lower_x, LINE_TOP, lower_x, LINE_BOTTOM, WHITE); DrawLine(upper_x, LINE_TOP, upper_x, LINE_BOTTOM, WHITE); + state.mutex.lock(); std::stack> stack; cw::node::Node n = state.allocator.get_val(0); stack.push(std::make_pair(0, n.value.norm)); @@ -87,6 +88,7 @@ void draw_tree(const DrawState &ds, const State &state) stack.push(std::make_pair(n.right, right.value.norm)); } } + state.mutex.unlock(); } using Clock = std::chrono::steady_clock; diff --git a/src/state.cpp b/src/state.cpp index 22f9572..3cd6e0a 100644 --- a/src/state.cpp +++ b/src/state.cpp @@ -12,6 +12,7 @@ namespace cw::state { void DrawState::compute_bounds() { + state.mutex.lock(); bounds.leftmost = state.allocator.get_val(0); while (bounds.leftmost.left >= 0) bounds.leftmost = state.allocator.get_val(bounds.leftmost.left); @@ -19,6 +20,7 @@ namespace cw::state bounds.rightmost = state.allocator.get_val(0); while (bounds.rightmost.right >= 0) bounds.rightmost = state.allocator.get_val(bounds.rightmost.right); + state.mutex.unlock(); bounds.lower_val = std::floorl(bounds.leftmost.value.norm); bounds.upper_val = std::ceill(bounds.rightmost.value.norm); diff --git a/src/state.hpp b/src/state.hpp index 3a26a10..5759830 100644 --- a/src/state.hpp +++ b/src/state.hpp @@ -29,14 +29,14 @@ namespace cw::state struct DrawState { - const State &state; + State &state; struct Bounds { cw::node::Node leftmost, rightmost; f64 lower_val, upper_val; } bounds; - DrawState(const State &state) : state{state} {}; + DrawState(State &state) : state{state} {}; void compute_bounds(void); };