john/McRogueFace

Fork 0

[Bugfix] GridLayer::parent_grid is a dangling raw pointer when grid is destroyed #270

New issue

Closed

opened 2026-03-07 23:22:36 +00:00 by john · 2 comments

john commented

2026-03-07 23:22:36 +00:00

Owner

Summary

GridLayer stores its parent grid as a raw UIGrid* pointer (GridLayers.h:34). When the owning grid is destroyed, this pointer is not automatically nullified. If a Python-side reference to the layer outlives the grid, any method that accesses parent_grid dereferences freed memory.

Root Cause

GridLayers.h:34:

class GridLayer {
    ...
    UIGrid* parent_grid;   // Parent grid reference
    ...
};

This raw pointer is used extensively in GridLayers.cpp:

Line 304, 309, 313: FOV computation via parent_grid->computeFOV()
Line 319: FOV query via parent_grid->isInFOV()
Line 361, 366, 367: Perspective update via parent_grid->fov_radius

While some access points check if (!parent_grid), the pointer itself becomes dangling (non-NULL but invalid) when the grid is destroyed — the check passes but the dereference is UB.

Reproduction

import mcrfpy

grid = mcrfpy.Grid(grid_size=(10, 10))
layer = mcrfpy.TileLayer(name="terrain", z_index=0, texture=some_texture)
grid.add_layer(layer)

# Keep reference to layer, destroy grid
del grid
# layer.parent_grid is now dangling

# Any operation that accesses parent_grid will crash
layer.grid = another_grid  # Setter reads parent_grid internally

Note: The grid setter code (GridLayers.cpp:1624, 2259) does check if (!self->data->parent_grid), but a dangling pointer won't be NULL — it will appear valid and dereference freed memory.

Fix Options

Use std::shared_ptr<UIGrid>: Keeps grid alive while layers reference it — but creates ownership issues since grid owns layers
Use std::weak_ptr<UIGrid>: Best option. Grid stays as owner, layers hold weak references. Check .lock() before access.
Invalidation on destruction: Grid destructor sets all layers' parent_grid = nullptr. Requires tracking.

Option 2 (weak_ptr) is cleanest: it mirrors the pattern already used by PyUniformCollectionObject::owner.

Severity

Medium — requires grid to be destroyed while layer references exist. Currently uncommon in practice because grids tend to live longer than their layers, but becomes important with dynamic scene management.

## Summary `GridLayer` stores its parent grid as a raw `UIGrid*` pointer (`GridLayers.h:34`). When the owning grid is destroyed, this pointer is not automatically nullified. If a Python-side reference to the layer outlives the grid, any method that accesses `parent_grid` dereferences freed memory. ## Root Cause `GridLayers.h:34`: ```cpp class GridLayer { ... UIGrid* parent_grid; // Parent grid reference ... }; ``` This raw pointer is used extensively in `GridLayers.cpp`: - Line 304, 309, 313: FOV computation via `parent_grid->computeFOV()` - Line 319: FOV query via `parent_grid->isInFOV()` - Line 361, 366, 367: Perspective update via `parent_grid->fov_radius` While some access points check `if (!parent_grid)`, the pointer itself becomes dangling (non-NULL but invalid) when the grid is destroyed — the check passes but the dereference is UB. ## Reproduction ```python import mcrfpy grid = mcrfpy.Grid(grid_size=(10, 10)) layer = mcrfpy.TileLayer(name="terrain", z_index=0, texture=some_texture) grid.add_layer(layer) # Keep reference to layer, destroy grid del grid # layer.parent_grid is now dangling # Any operation that accesses parent_grid will crash layer.grid = another_grid # Setter reads parent_grid internally ``` **Note**: The grid setter code (`GridLayers.cpp:1624, 2259`) does check `if (!self->data->parent_grid)`, but a dangling pointer won't be NULL — it will appear valid and dereference freed memory. ## Fix Options 1. **Use `std::shared_ptr<UIGrid>`**: Keeps grid alive while layers reference it — but creates ownership issues since grid owns layers 2. **Use `std::weak_ptr<UIGrid>`**: Best option. Grid stays as owner, layers hold weak references. Check `.lock()` before access. 3. **Invalidation on destruction**: Grid destructor sets all layers' `parent_grid = nullptr`. Requires tracking. Option 2 (weak_ptr) is cleanest: it mirrors the pattern already used by `PyUniformCollectionObject::owner`. ## Severity **Medium** — requires grid to be destroyed while layer references exist. Currently uncommon in practice because grids tend to live longer than their layers, but becomes important with dynamic scene management.

john referenced this issue

2026-03-07 23:25:45 +00:00

[Bugfix] GridChunk::parent_grid raw pointer can dangle #277

john referenced this issue

2026-03-07 23:26:21 +00:00

[Meta] Engine memory safety audit — 7DRL 2026 post-mortem #279

john added the

Bugfix

label

2026-03-08 00:01:04 +00:00

john added a new dependency

2026-03-08 21:07:33 +00:00

#252 GridView and Grid, FOVLayer and PathLayer

john commented

2026-03-08 21:07:50 +00:00

Author

Owner

Deferred from the #258–#278 memory safety audit. The raw UIGrid* pointer issue is naturally resolved by #252 (GridMap/GridView split), which replaces UIGrid entirely. GridLayer::parent_grid will change from UIGrid* to GridMap* with proper lifetime management as part of that architecture change. Linked as dependency of #252.

Deferred from the #258–#278 memory safety audit. The raw `UIGrid*` pointer issue is naturally resolved by #252 (GridMap/GridView split), which replaces `UIGrid` entirely. GridLayer::parent_grid will change from `UIGrid*` to `GridMap*` with proper lifetime management as part of that architecture change. Linked as dependency of #252.

john referenced this issue

2026-03-15 20:17:53 +00:00

GridView and Grid, FOVLayer and PathLayer #252

john commented

2026-03-15 20:18:09 +00:00

Author

Owner

Roadmap context

Folded into #252 (Grid/GridView split) in the Grid & Entity Overhaul Roadmap (docs/GRID_ENTITY_OVERHAUL_ROADMAP.md), Phase 4. During the split, all raw UIGrid* pointers in GridLayer, UIGridPoint, and GridChunk will be converted to weak_ptr<Grid>.

## Roadmap context Folded into #252 (Grid/GridView split) in the Grid & Entity Overhaul Roadmap (`docs/GRID_ENTITY_OVERHAUL_ROADMAP.md`), **Phase 4**. During the split, all raw `UIGrid*` pointers in GridLayer, UIGridPoint, and GridChunk will be converted to `weak_ptr<Grid>`.

john referenced this issue from a commit

2026-03-16 22:23:55 +00:00

Phase 4.1: Extract GridData base class from UIGrid (#252, #270, #271, #277)

john referenced this issue

2026-04-04 08:34:50 +00:00

GridView and Grid, FOVLayer and PathLayer #252

john referenced this issue from a commit

2026-04-10 05:09:18 +00:00

Add issue triage document for all 46 open issues

john referenced this issue

2026-04-10 05:35:16 +00:00

[Meta] Engine memory safety audit — 7DRL 2026 post-mortem #279