Level: relaible freeing

entity.c

@@ -5,9 +5,9 @@
 
 #define ENTITY_DEBUG(...) FURI_LOG_D("Entity", __VA_ARGS__)
 
-static size_t entities_count = 0;
+static int32_t entities_count = 0;
 
-size_t entities_get_count(void) {
+int32_t entities_get_count(void) {
     return entities_count;
 }
 
@@ -26,13 +26,13 @@ Entity* entity_alloc(const EntityDescription* behaviour) {
     if(behaviour && behaviour->context_size > 0) {
         entity->context = malloc(behaviour->context_size);
     }
-    ENTITY_DEBUG("Allocated entity at %p", entity);
+    ENTITY_DEBUG("Allocated at %p", entity);
     return entity;
 }
 
 void entity_free(Entity* entity) {
     entities_count--;
-    ENTITY_DEBUG("Freeing entity at %p", entity);
+    ENTITY_DEBUG("Freeing at %p", entity);
     if(entity->context) {
         free(entity->context);
     }
@@ -51,15 +51,15 @@ void* entity_context_get(Entity* entity) {
     return entity->context;
 }
 
-void entity_call_start(Entity* entity) {
+void entity_call_start(Level* level, Entity* entity) {
     if(entity->behaviour && entity->behaviour->start) {
-        entity->behaviour->start(entity->context);
+        entity->behaviour->start(entity, level, entity->context);
     }
 }
 
-void entity_call_stop(Entity* entity) {
+void entity_call_stop(Level* level, Entity* entity) {
     if(entity->behaviour && entity->behaviour->stop) {
-        entity->behaviour->stop(entity->context);
+        entity->behaviour->stop(entity, level, entity->context);
     }
 }
 

entity.h

@@ -17,8 +17,8 @@ typedef struct {
 #define VECTOR_ZERO ((Vector){0, 0})
 
 typedef struct {
-    void (*start)(void* context);
-    void (*stop)(void* context);
+    void (*start)(Entity* entity, Level* level, void* context);
+    void (*stop)(Entity* entity, Level* level, void* context);
     void (*update)(Entity* entity, Director* director, void* context);
     void (*render)(Entity* entity, Director* director, Canvas* canvas, void* context);
     size_t context_size;

entity_i.h

@@ -10,15 +10,15 @@ Entity* entity_alloc(const EntityDescription* behaviour);
 
 void entity_free(Entity* entity);
 
-void entity_call_start(Entity* entity);
+void entity_call_start(Level* level, Entity* entity);
 
-void entity_call_stop(Entity* entity);
+void entity_call_stop(Level* level, Entity* entity);
 
 void entity_call_update(Entity* entity, Director* director);
 
 void entity_call_render(Entity* entity, Director* director, Canvas* canvas);
 
-size_t entities_get_count(void);
+int32_t entities_get_count(void);
 
 #ifdef __cplusplus
 }

level.c

@@ -5,78 +5,96 @@
 #include <furi.h>
 
 LIST_DEF(EntityList, Entity*, M_POD_OPLIST);
+#define M_OPL_EntityList_t() LIST_OPLIST(EntityList)
+#define FOREACH(name, list) for \
+    M_EACH(name, list, EntityList_t)
+
+#define LEVEL_DEBUG(...) FURI_LOG_D("Level", __VA_ARGS__)
+#define LEVEL_ERROR(...) FURI_LOG_E("Level", __VA_ARGS__)
 
 struct Level {
     EntityList_t entities;
-    EntityList_t entities_to_add;
-    EntityList_t entities_to_remove;
+    EntityList_t to_add;
+    EntityList_t to_remove;
     LevelBehaviour behaviour;
     void* context;
 };
 
-static void level_process_add(Level* level) {
-    EntityList_it_t it;
-    EntityList_it(it, level->entities_to_add);
-    while(!EntityList_end_p(it)) {
-        EntityList_push_back(level->entities, *EntityList_cref(it));
-        entity_call_start(*EntityList_ref(it));
-        EntityList_next(it);
-    }
-    EntityList_clear(level->entities_to_add);
+Level* level_alloc(void) {
+    Level* level = malloc(sizeof(Level));
+    EntityList_init(level->entities);
+    EntityList_init(level->to_add);
+    EntityList_init(level->to_remove);
+    level->behaviour = LEVEL_BEHAVIOUR_EMPTY;
+    level->context = NULL;
+    LEVEL_DEBUG("Allocated level at %p", level);
+    return level;
 }
 
-static void level_find_and_remove(Level* level, Entity* entity) {
-    EntityList_it_t it;
-    EntityList_it(it, level->entities);
-    while(!EntityList_end_p(it)) {
-        if(*EntityList_ref(it) == entity) {
-            entity_call_stop(*EntityList_ref(it));
-            EntityList_remove(level->entities, it);
-            entity_free(entity);
-            break;
-        }
-        EntityList_next(it);
+static void level_process_add(Level* level) {
+    // move entities from to_add to entities
+    FOREACH(item, level->to_add) {
+        EntityList_push_back(level->entities, *item);
     }
+    EntityList_clear(level->to_add);
 }
 
 static void level_process_remove(Level* level) {
-    EntityList_it_t it;
-    EntityList_it(it, level->entities_to_remove);
-    while(!EntityList_end_p(it)) {
-        level_find_and_remove(level, *EntityList_cref(it));
-        EntityList_next(it);
+    // remove entities in to_remove from entities and free them
+    FOREACH(item, level->to_remove) {
+        entity_free(*item);
+        EntityList_it_t it;
+
+        // find and remove the entity from the entities list
+        for(EntityList_it(it, level->entities); !EntityList_end_p(it); EntityList_next(it)) {
+            if(*EntityList_ref(it) == *item) {
+                EntityList_remove(level->entities, it);
+                break;
+            }
+        }
     }
-    EntityList_clear(level->entities_to_remove);
+    EntityList_clear(level->to_remove);
 }
 
-Level* level_alloc(void) {
-    Level* level = malloc(sizeof(Level));
-    EntityList_init(level->entities);
-    EntityList_init(level->entities_to_add);
-    EntityList_init(level->entities_to_remove);
-    level->behaviour = LEVEL_BEHAVIOUR_EMPTY;
-    level->context = NULL;
-    FURI_LOG_D("Level", "Allocated level at %p", level);
-    return level;
+static bool level_entity_in_list_p(EntityList_t list, Entity* entity) {
+    FOREACH(item, list) {
+        if(*item == entity) {
+            return true;
+        }
+    }
+    return false;
 }
 
 void level_free(Level* level) {
-    level_process_add(level);
-    level_process_remove(level);
+    size_t iterations = 0;
+
+    do {
+        // process to_add and to_remove
+        level_process_add(level);
+        level_process_remove(level);
+
+        // remove entities from entities list
+        FOREACH(item, level->entities) {
+            if(!level_entity_in_list_p(level->to_remove, *item)) {
+                level_remove_entity(level, *item);
+            }
+        }
 
-    EntityList_it_t it;
-    EntityList_it(it, level->entities);
-    while(!EntityList_end_p(it)) {
-        entity_call_stop(*EntityList_ref(it));
-        entity_free(*EntityList_ref(it));
-        EntityList_next(it);
-    }
+        // check if we are looping too many times
+        iterations++;
+        if(iterations >= 100) {
+            LEVEL_ERROR("Level free looped too many times");
+        }
+
+        // entity_call_stop can call level_remove_entity or level_add_entity
+        // so we need to process to_add and to_remove again
+    } while(!EntityList_empty_p(level->to_add) || !EntityList_empty_p(level->to_remove));
 
     EntityList_clear(level->entities);
-    EntityList_clear(level->entities_to_add);
-    EntityList_clear(level->entities_to_remove);
+    EntityList_clear(level->to_add);
+    EntityList_clear(level->to_remove);
 
-    FURI_LOG_D("Level", "Freeing level at %p", level);
+    LEVEL_DEBUG("Freeing level at %p", level);
     free(level);
 }
 
@@ -87,31 +105,27 @@ void level_behaviour_set(Level* level, LevelBehaviour behaviour, void* context)
 
 Entity* level_add_entity(Level* level, const EntityDescription* behaviour) {
     Entity* entity = entity_alloc(behaviour);
-    EntityList_push_back(level->entities_to_add, entity);
+    EntityList_push_back(level->to_add, entity);
+    entity_call_start(level, entity);
     return entity;
 }
 
 void level_remove_entity(Level* level, Entity* entity) {
-    EntityList_push_back(level->entities_to_remove, entity);
+    EntityList_push_back(level->to_remove, entity);
+    entity_call_stop(level, entity);
 }
 
 void level_update(Level* level, Director* director) {
     level_process_add(level);
     level_process_remove(level);
 
-    EntityList_it_t it;
-    EntityList_it(it, level->entities);
-    while(!EntityList_end_p(it)) {
-        entity_call_update(*EntityList_ref(it), director);
-        EntityList_next(it);
+    FOREACH(item, level->entities) {
+        entity_call_update(*item, director);
     }
 }
 
 void level_render(Level* level, Director* director, Canvas* canvas) {
-    EntityList_it_t it;
-    EntityList_it(it, level->entities);
-    while(!EntityList_end_p(it)) {
-        entity_call_render(*EntityList_ref(it), director, canvas);
-        EntityList_next(it);
+    FOREACH(item, level->entities) {
+        entity_call_render(*item, director, canvas);
     }
 }

main.c

@@ -44,9 +44,9 @@ int32_t game_app(void* p) {
     level_manager_free(level_manager);
     director_free(director);
 
-    size_t entities = entities_get_count();
+    int32_t entities = entities_get_count();
     if(entities != 0) {
-        FURI_LOG_E("Game", "Memory leak detected: %d entities still allocated", entities);
+        FURI_LOG_E("Game", "Memory leak detected: %ld entities still allocated", entities);
         return -1;
     }