[pjs] Fix a memory leak in the PipyJS engine

src/api/algo.cpp

@@ -59,20 +59,16 @@ Cache::Options::Options(pjs::Object *options) {
 // Cache
 //
 
-static int s_count = 0;
-
 Cache::Cache(const Options &options, pjs::Function *allocate, pjs::Function *free)
   : m_options(options)
   , m_allocate(allocate)
   , m_free(free)
   , m_cache(pjs::OrderedHash<pjs::Value, Entry>::make())
 {
-  // printf("cache count = %d\n", ++s_count);
   m_options.ttl *= 1000;
 }
 
 Cache::~Cache() {
-  // printf("cache count = %d\n", --s_count);
 }
 
 bool Cache::get(pjs::Context &ctx, const pjs::Value &key, pjs::Value &value) {

src/api/stats.cpp

@@ -84,15 +84,12 @@ auto Metric::local() -> MetricSet& {
   return s_local_metric_set;
 }
 
-static int s_count = 0;
-
 Metric::Metric(pjs::Str *name, pjs::Array *label_names, MetricSet *set)
   : m_root(nullptr)
   , m_name(name)
   , m_label_index(-1)
   , m_label_names(std::make_shared<std::vector<pjs::Ref<pjs::Str>>>())
 {
-  // printf("metric count = %d\n", ++s_count);
   if (label_names) {
     std::string shape;
     auto n = label_names->length();
@@ -124,16 +121,11 @@ Metric::Metric(Metric *parent, pjs::Str **labels)
   , m_label_index(parent->m_label_index + 1)
   , m_label_names(parent->m_label_names)
 {
-  // printf("metric count = %d\n", ++s_count);
   parent->m_subs.emplace_back();
   parent->m_subs.back() = this;
   parent->m_sub_map[m_label] = this;
 }
 
-Metric::~Metric() {
-  // printf("metric count = %d\n", --s_count);
-}
-
 auto Metric::with_labels(pjs::Str *const *labels, int count) -> Metric* {
   int num_labels = m_label_names->size();
   if (m_label_index + 1 >= num_labels) {

src/api/stats.hpp

@@ -68,7 +68,7 @@ class Metric : public pjs::ObjectTemplate<Metric> {
 protected:
   Metric(pjs::Str *name, pjs::Array *label_names, MetricSet *set = nullptr);
   Metric(Metric *parent, pjs::Str **labels);
-  virtual ~Metric(); // {}
+  virtual ~Metric() {}
 
   bool has_value() const { return m_has_value; }
   void create_value();

src/pjs/types.cpp

@@ -541,19 +541,43 @@ void Context::backtrace(const std::string &name) {
 }
 
 //
-// Class
+// ClassMap
 //
 
-thread_local std::map<std::string, Class*> Class::m_class_map;
-thread_local std::vector<Class::ClassSlot> Class::m_class_slots(1);
-thread_local size_t Class::m_class_slot_free = 0;
+thread_local Ref<ClassMap> ClassMap::m_singleton;
+
+auto ClassMap::add(Class *c) -> size_t {
+  auto id = m_class_slot_free;
+  if (!id) {
+    id = m_class_slots.size();
+    m_class_slots.push_back({ c });
+  } else {
+    m_class_slot_free = m_class_slots[id].next_slot;
+    m_class_slots[id].class_ptr = c;
+  }
+  if (c->name() != Str::empty) m_class_map[c->name()->str()] = c;
+  return id;
+}
+
+void ClassMap::remove(Class *c) {
+  if (c->name() != Str::empty) m_class_map.erase(c->name()->str());
+  auto &slot = m_class_slots[c->id()];
+  slot.class_ptr = nullptr;
+  slot.next_slot = m_class_slot_free;
+  m_class_slot_free = c->id();
+}
+
+//
+// Class
+//
 
 Class::Class(
   const std::string &name,
   Class *super,
   const std::list<Field*> &fields)
   : m_super(super)
   , m_name(pjs::Str::make(name))
+  , m_class_map(ClassMap::get())
 {
   if (super) {
     m_field_map = super->m_field_map;
@@ -583,26 +607,14 @@ Class::Class(
     }
   }
   for (auto &p : m_field_map) p.first->retain();
-  if (!name.empty()) m_class_map[name] = this;
-  if (auto id = m_class_slot_free) {
-    m_id = id;
-    m_class_slot_free = m_class_slots[id].next_slot;
-    m_class_slots[id].class_ptr = this;
-  } else {
-    m_id = m_class_slots.size();
-    m_class_slots.push_back({ this });
-  }
+  m_id = m_class_map->add(this);
 }
 
 Class::~Class() {
-  if (!m_name->str().empty()) m_class_map.erase(m_name->str());
-  auto &slot = m_class_slots[m_id];
-  slot.class_ptr = nullptr;
-  slot.next_slot = m_class_slot_free;
-  m_class_slot_free = m_id;
   for (auto &p : m_field_map) {
     p.first->release();
   }
+  if (m_class_map) m_class_map->remove(this);
 }
 
 void Class::assign(Object *obj, Object *src) {

src/pjs/types.hpp

@@ -886,29 +886,67 @@ class Field : public RefCount<Field> {
   friend class RefCount<Field>;
 };
 
+//
+// ClassMap
+//
+
+class ClassMap : public RefCount<ClassMap> {
+public:
+  static auto get() -> ClassMap* {
+    if (!m_singleton) {
+      m_singleton = new ClassMap;
+    }
+    return m_singleton;
+  }
+
+  auto get(const std::string &name) -> Class* {
+    auto i = m_class_map.find(name);
+    return i == m_class_map.end() ? nullptr : i->second;
+  }
+
+  auto get(size_t id) -> Class* {
+    if (id >= m_class_slots.size()) return nullptr;
+    return m_class_slots[id].class_ptr;
+  }
+
+  auto all() -> const std::map<std::string, Class*>& { return m_class_map; }
+
+  auto add(Class *c) -> size_t;
+  void remove(Class *c);
+
+private:
+  struct Slot {
+    Class *class_ptr;
+    size_t next_slot;
+  };
+
+  std::map<std::string, Class*> m_class_map;
+  std::vector<Slot> m_class_slots;
+  size_t m_class_slot_free = 0;
+
+  thread_local static Ref<ClassMap> m_singleton;
+};
+
 //
 // Class
 //
 
-class Class :
-  public RefCount<Class>,
-  public Pooled<Class> // TODO: crashes when reload on Ubuntu without being pooled, why?
-{
+class Class : public RefCount<Class> {
 public:
   static auto make(const std::string &name, Class *super, const std::list<Field*> &fields) -> Class* {
     return new Class(name, super, fields);
   }
 
-  static auto all() -> const std::map<std::string, Class*>& { return m_class_map; }
+  static auto all() -> const std::map<std::string, Class*>& {
+    return ClassMap::get()->all();
+  }
 
   static auto get(const std::string &name) -> Class* {
-    auto i = m_class_map.find(name);
-    return i == m_class_map.end() ? nullptr : i->second;
+    return ClassMap::get()->get(name);
   }
 
   static auto get(size_t id) -> Class* {
-    if (id >= m_class_slots.size()) return nullptr;
-    return m_class_slots[id].class_ptr;
+    return ClassMap::get()->get(id);
   }
 
   auto name() const -> pjs::Str* { return m_name; }
@@ -967,7 +1005,8 @@ class Class :
   ~Class();
 
   Class* m_super = nullptr;
-  pjs::Ref<pjs::Str> m_name;
+  Ref<Str> m_name;
+  Ref<ClassMap> m_class_map;
   std::function<Object*(Context&)> m_ctor;
   std::function<void(Object*, int, Value&)> m_geti;
   std::function<void(Object*, int, const Value&)> m_seti;
@@ -978,15 +1017,6 @@ class Class :
   size_t m_id;
   size_t m_object_count = 0;
 
-  struct ClassSlot {
-    Class *class_ptr;
-    size_t next_slot;
-  };
-
-  thread_local static std::map<std::string, Class*> m_class_map;
-  thread_local static std::vector<ClassSlot> m_class_slots;
-  thread_local static size_t m_class_slot_free;
-
   friend class RefCount<Class>;
 };
 
@@ -1014,7 +1044,6 @@ class ClassDef {
       }
       delete m_init_data;
       m_init_data = nullptr;
-      m_c->retain();
     }
     return m_c;
   }
@@ -1060,14 +1089,14 @@ class ClassDef {
     std::function<void(Object*, int, const Value&)> seti;
   };
 
-  thread_local static Class* m_c;
+  thread_local static Ref<Class> m_c;
   thread_local static InitData* m_init_data;
 };
 
 template<class T>
 Class* class_of() { return ClassDef<T>::get(); }
 
-template<class T> thread_local Class* ClassDef<T>::m_c = nullptr;
+template<class T> thread_local Ref<Class> ClassDef<T>::m_c;
 template<class T> thread_local typename ClassDef<T>::InitData* ClassDef<T>::m_init_data = nullptr;
 
 template<class T>
@@ -1794,8 +1823,8 @@ inline auto Class::init(Object *obj, Object *prototype) -> Object* {
 
 inline void Class::free(Object *obj) {
   obj->m_data->free();
-  release();
   m_object_count--;
+  release();
 }
 
 inline bool Object::has(Str *key) {