Weird bug in draw_tree

Seems near the 100,000 node mark, the vector craps itself?  I keep
getting "heap use after free" errors here when trying to access the
allocator vector.  Disabling fsan just leads to a segment fault near
the same point.

I think we might need to introduce our own vector :)

build.sh

@@ -8,7 +8,7 @@ DFLAGS="-ggdb -fsanitize=address -fsanitize=undefined"
 CFLAGS="$GFLAGS $DFLAGS"
 LIBS="-lraylib -lm"
 
-c++ $CFLAGS -o $OUT src/node.cpp src/state.cpp src/worker.cpp src/numerics.cpp src/main.cpp $LIBS
+c++ $CFLAGS -o $OUT src/node.cpp src/state.cpp src/worker.cpp src/main.cpp $LIBS
 
 if [ "$1" = "run" ]
 then

src/main.cpp

@@ -18,7 +18,6 @@
 
 #include "base.hpp"
 #include "node.hpp"
-#include "numerics.hpp"
 #include "worker.hpp"
 
 #define WIDTH       1024
@@ -31,14 +30,14 @@
 using cw::state::DrawState;
 using cw::state::State;
 
-std::pair<std::string, int> fraction_to_string(Fraction f)
+std::pair<std::string, int> fraction_to_string(cw::node::Fraction f)
 {
   std::string s{to_string(f)};
   int width = MeasureText(s.c_str(), FONT_SIZE);
   return std::make_pair(s, width);
 }
 
-void draw_fraction(Fraction f, word_t x, word_t y)
+void draw_fraction(cw::node::Fraction f, u64 x, u64 y)
 {
   std::string s;
   int width;
@@ -53,7 +52,7 @@ constexpr u64 clamp_to_width(const DrawState &ds, f64 val)
          (val - ds.bounds.lower_val);
 }
 
-void draw_tree(const DrawState &ds)
+void draw_tree(const DrawState &ds, const State &state)
 {
   // Number line
   DrawLine(0, HEIGHT / 2, WIDTH, HEIGHT / 2, WHITE);
@@ -64,18 +63,29 @@ void draw_tree(const DrawState &ds)
   DrawLine(lower_x, LINE_TOP, lower_x, LINE_BOTTOM, WHITE);
   DrawLine(upper_x, LINE_TOP, upper_x, LINE_BOTTOM, WHITE);
 
-  std::stack<cw::node::Node> stack;
-  stack.push(ds.state.allocator.get_val(0));
+  std::stack<std::pair<u64, f64>> stack;
+  cw::node::Node n = state.allocator.get_val(0);
+  stack.push(std::make_pair(0, n.value.norm));
   while (!stack.empty())
   {
-    auto n = stack.top();
+    u64 node_index;
+    f64 norm;
+    std::tie(node_index, norm) = stack.top();
     stack.pop();
-    u64 x = clamp_to_width(ds, n.value.norm);
+    u64 x = clamp_to_width(ds, norm);
     DrawLine(x, LINE_TOP, x, LINE_BOTTOM, RED);
+
+    cw::node::Node n = state.allocator.get_val(node_index);
     if (n.left >= 0)
-      stack.push(ds.state.allocator.get_val(n.left));
+    {
+      cw::node::Node left = state.allocator.get_val(n.left);
+      stack.push(std::make_pair(n.left, left.value.norm));
+    }
     if (n.right >= 0)
-      stack.push(ds.state.allocator.get_val(n.right));
+    {
+      cw::node::Node right = state.allocator.get_val(n.right);
+      stack.push(std::make_pair(n.right, right.value.norm));
+    }
   }
 }
 
@@ -84,14 +94,19 @@ using Ms    = std::chrono::milliseconds;
 
 int main(void)
 {
-  // Init general state
-  cw::state::State state;
-  cw::state::DrawState draw_state{state};
-
   // Init timer
   auto time_current         = Clock::now();
   auto time_previous        = time_current;
-  constexpr auto time_delta = 1;
+  constexpr auto time_delta = 0;
+
+  // Init general state
+  cw::state::State state;
+  state.stop_work  = false;
+  state.pause_work = false;
+  state.allocator.alloc(cw::node::Node{{1, 1}, -1, -1});
+  state.queue.push(0);
+
+  cw::state::DrawState draw_state{state};
 
   // Init meta text (counter, iterations, etc)
   u64 count = 1, prev_count = 0;
@@ -99,11 +114,7 @@ int main(void)
   std::string format_str;
   u64 format_str_width = 0;
 
-  // Setup our first node (1/1)
-  state.allocator.alloc(cw::node::Node{{1, 1}, -1, -1});
-  state.queue.push(0);
-
-  // Init thread
+  // Init threads
   std::thread threads[] = {
       std::thread(cw::worker::worker, std::ref(state)),
       std::thread(cw::worker::worker, std::ref(state)),
@@ -119,12 +130,12 @@ int main(void)
   {
     // Update
     time_current = Clock::now();
-    count        = state.allocator.vec.size();
     if (!state.pause_work &&
         std::chrono::duration_cast<Ms>(time_current - time_previous).count() >=
             time_delta)
     {
       time_previous = time_current;
+      count         = state.allocator.vec.size();
     }
 
     if (prev_count != count)
@@ -145,7 +156,7 @@ int main(void)
 
     ClearBackground(BLACK);
     BeginDrawing();
-    draw_tree(draw_state);
+    draw_tree(draw_state, state);
     DrawText(format_str.c_str(), WIDTH / 2 - format_str_width / 2, HEIGHT / 8,
              FONT_SIZE, WHITE);
     EndDrawing();

src/numerics.cpp

@@ -1,153 +0,0 @@
-/* numerics.cpp: Implementation of numerics
- * Created: 2024-07-26
- * Author: Aryadev Chavali
- * License: See end of file
- * Commentary:
- */
-
-#include "./numerics.hpp"
-
-#include <sstream>
-
-Fraction::Fraction(word_t numerator, word_t denominator)
-    : numerator{numerator}, denominator{denominator},
-      norm{numerator / ((long double)denominator)}
-{
-  word_t hcf = gcd(MIN(numerator, denominator), MAX(numerator, denominator));
-  numerator /= hcf;
-  denominator /= hcf;
-}
-
-// floating point arithmetic inaccuracies blah blah blah better to use
-// simplified fractions here
-
-bool Fraction::operator<(const Fraction other)
-{
-  if (other.denominator == denominator)
-    return numerator < other.numerator;
-  // TODO: Is it better to use the GCD?
-  return (numerator * other.denominator) < (other.numerator * denominator);
-}
-
-bool Fraction::operator==(const Fraction &other)
-{
-  return numerator == other.numerator && denominator == other.denominator;
-}
-
-Node::Node(Fraction val, index_t left, index_t right)
-    : value{val}, left{left}, right{right}
-{
-}
-
-NodeAllocator::NodeAllocator(word_t capacity)
-{
-  vec.reserve(capacity);
-}
-
-word_t NodeAllocator::alloc(Node n)
-{
-  word_t ind = vec.size();
-  vec.push_back(n);
-  return ind;
-}
-
-// WHY DO I NEED TO DO IT TWICE REEEEEEE
-Node &NodeAllocator::getRef(word_t n)
-{
-  if (n >= vec.size())
-    return vec[0];
-  return vec[n];
-}
-
-Node NodeAllocator::getVal(word_t n) const
-{
-  if (n >= vec.size())
-    return vec[0];
-  return vec[n];
-}
-
-word_t gcd(word_t a, word_t b)
-{
-  if (a == b)
-    return a;
-  else if (a <= 1 || b <= 1)
-    return 1;
-  for (word_t r = b % a; r != 0; b = a, a = r, r = b % a)
-    continue;
-  return a;
-}
-
-std::tuple<Fraction, Fraction, Fraction> iterate(std::queue<word_t> &queue,
-                                                 NodeAllocator &allocator)
-{
-  if (queue.empty())
-    return {};
-  word_t index = queue.front();
-  Node node    = allocator.getVal(index);
-  if (!node.left.has_value())
-  {
-    allocator.getRef(index).left = allocator.alloc(Fraction{
-        node.value.numerator, node.value.numerator + node.value.denominator});
-  }
-  if (!node.right.has_value())
-  {
-    allocator.getRef(index).right = allocator.alloc(Fraction{
-        node.value.numerator + node.value.denominator, node.value.denominator});
-  }
-  queue.pop();
-  queue.push(allocator.getVal(index).left.value());
-  queue.push(allocator.getVal(index).right.value());
-  node = allocator.getVal(index);
-  // NOTE: We can be assured that left and right DO have values
-  return std::tuple(allocator.getVal(node.left.value()).value, node.value,
-                    allocator.getVal(node.right.value()).value);
-}
-
-std::string to_string(const Fraction &f)
-{
-  std::stringstream ss;
-  ss << f.numerator << "/" << f.denominator;
-  return ss.str();
-}
-
-void indent_depth(int depth, std::stringstream &ss)
-{
-  for (int i = 0; i < depth; ++i)
-    ss << "  ";
-}
-
-std::string to_string(const NodeAllocator &allocator, const index_t n,
-                      int depth)
-{
-  if (!n.has_value())
-    return "NIL";
-  std::stringstream ss;
-  Node x = allocator.getVal(n.value());
-  ss << "(" << to_string(x.value) << "\n";
-  indent_depth(depth, ss);
-  if (x.left == -1)
-    ss << "NIL";
-  else
-    ss << to_string(allocator, x.left, depth + 1);
-  ss << "\n";
-  indent_depth(depth, ss);
-  if (x.right == -1)
-    ss << "NIL";
-  else
-    ss << to_string(allocator, x.right, depth + 1);
-  ss << ")";
-  return ss.str();
-}
-
-/* Copyright (C) 2024, 2025 Aryadev Chavali
-
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
- * FOR A PARTICULAR PURPOSE.  See the GNU General Public License Version 2 for
- * details.
-
- * You may distribute and modify this code under the terms of the GNU General
- * Public License Version 2, which you should have received a copy of along with
- * this program.  If not, please go to <https://www.gnu.org/licenses/>.
-
- */

src/numerics.hpp

@@ -1,72 +0,0 @@
-/* numerics.hpp: Computation necessary for generating the tree
- * Created: 2024-07-26
- * Author: Aryadev Chavali
- * License: See end of file
- * Commentary:
- */
-
-#ifndef NUMERICS_HPP
-#define NUMERICS_HPP
-
-#include <cstdint>
-
-#include <optional>
-#include <queue>
-#include <string>
-#include <tuple>
-#include <vector>
-
-#define MIN(A, B) ((A) < (B) ? (A) : (B))
-#define MAX(A, B) ((B) < (A) ? (A) : (B))
-typedef uint64_t word_t;
-typedef std::optional<word_t> index_t;
-
-struct Fraction
-{
-  word_t numerator, denominator;
-  long double norm;
-
-  Fraction(word_t numerator = 0, word_t denominator = 1);
-  bool operator<(const Fraction other);
-  bool operator==(const Fraction &other);
-};
-
-struct Node
-{
-  Fraction value;
-  index_t left, right;
-
-  Node(Fraction val = {}, index_t left = std::nullopt,
-       index_t right = std::nullopt);
-};
-
-struct NodeAllocator
-{
-  std::vector<Node> vec;
-
-  NodeAllocator(word_t capacity = 0);
-  word_t alloc(Node n);
-  Node getVal(word_t n) const;
-  Node &getRef(word_t n);
-};
-
-word_t gcd(word_t a, word_t b);
-std::tuple<Fraction, Fraction, Fraction> iterate(std::queue<word_t> &queue,
-                                                 NodeAllocator &allocator);
-
-std::string to_string(const Fraction &);
-std::string to_string(const NodeAllocator &, const index_t, int depth = 1);
-
-#endif
-
-/* Copyright (C) 2024, 2025 Aryadev Chavali
-
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
- * FOR A PARTICULAR PURPOSE.  See the GNU General Public License Version 2 for
- * details.
-
- * You may distribute and modify this code under the terms of the GNU General
- * Public License Version 2, which you should have received a copy of along with
- * this program.  If not, please go to <https://www.gnu.org/licenses/>.
- */

src/worker.cpp

@@ -16,42 +16,40 @@ namespace cw::worker
   using cw::node::Fraction;
   using cw::node::Node;
 
-  std::tuple<u64, u64> generate_children(NodeAllocator &allocator, u64 index)
-  {
-    Node node = allocator.get_val(index);
-    if (node.left < 0)
-    {
-      allocator.get_ref(index).left = allocator.alloc(Fraction{
-          node.value.numerator, node.value.numerator + node.value.denominator});
-    }
-    if (node.right < 0)
-    {
-      allocator.get_ref(index).right = allocator.alloc(
-          Fraction{node.value.numerator + node.value.denominator,
-                   node.value.denominator});
-    }
-    return {node.left, node.right};
-  }
-
   void do_iteration(State &state)
   {
-    // state.mutex.lock();
+    state.mutex.lock();
     if (state.queue.empty())
     {
       // Unlock since there isn't any work to be done.
-      // state.mutex.unlock();
+      state.mutex.unlock();
       return;
     }
     u64 index = state.queue.front();
     state.queue.pop();
 
-    u64 left_child, right_child;
-    std::tie(left_child, right_child) =
-        generate_children(state.allocator, index);
+    Node node = state.allocator.get_val(index);
 
-    state.queue.push(left_child);
-    state.queue.push(right_child);
-    // state.mutex.unlock();
+    i64 left = node.left, right = node.right;
+    if (left < 0)
+    {
+      left = state.allocator.alloc(Fraction{
+          node.value.numerator, node.value.numerator + node.value.denominator});
+    }
+    if (right < 0)
+    {
+      right = state.allocator.alloc(
+          Fraction{node.value.numerator + node.value.denominator,
+                   node.value.denominator});
+    }
+
+    Node &node_ref = state.allocator.get_ref(index);
+    node_ref.left  = left;
+    node_ref.right = right;
+
+    state.queue.push(left);
+    state.queue.push(right);
+    state.mutex.unlock();
   }
 
   void worker(State &state)

src/worker.hpp

@@ -18,11 +18,7 @@ namespace cw::worker
   using cw::node::NodeAllocator;
   using cw::state::State;
   constexpr auto THREAD_PAUSE_DELAY   = std::chrono::milliseconds(1000);
-  constexpr auto THREAD_GENERAL_DELAY = std::chrono::nanoseconds(100);
-
-  // Given `index`, return the indices of its children in the tree.  If not
-  // present already, generate them using the allocator.
-  std::tuple<u64, u64> generate_children(NodeAllocator &allocator, u64 index);
+  constexpr auto THREAD_GENERAL_DELAY = std::chrono::milliseconds(1);
 
   // Performs a single iteration which consists of the following:
   // 1) pop an index off the iteration queue