john/McRogueFace

Fork 0

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

New issue

Closed

opened 2026-03-07 23:25:45 +00:00 by john · 2 comments

john commented

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

Owner

Summary

GridChunk stores its parent grid as a raw UIGrid* pointer at GridChunk.h:39. This is the same class of bug as GridLayer::parent_grid (#270) — the raw pointer can dangle if the grid is destroyed while chunks are still referenced.

Root Cause

GridChunk.h:39:

class GridChunk {
    // ...
    UIGrid* parent_grid;   // Raw pointer
    // ...
};

And the parallel structure at line 72:

// Another GridChunk variant (likely for entity layer)
UIGrid* parent_grid;   // Also raw

Risk

GridChunks are owned by the grid, so under normal operation they don't outlive their parent. However:

If any code path takes a pointer/reference to a GridChunk and the grid is destroyed, the parent_grid pointer dangles
The inconsistency between raw pointers (GridChunk, GridLayer, UIGridPoint) and shared_ptr patterns used elsewhere makes the codebase harder to reason about

Fix

Same as #270 — convert to std::weak_ptr<UIGrid> or document the ownership invariant. Since GridChunks are always owned by grids and never exposed to Python, this is lower priority than the GridLayer and UIGridPoint cases.

Severity

Low — GridChunks are internal to UIGrid and not exposed to Python, so they generally can't outlive their parent grid. Fixing this is more about consistency than preventing crashes.

#270 — GridLayer::parent_grid (same pattern, higher risk because layers are Python-accessible)
#271 — UIGridPoint::parent_grid (same pattern)

## Summary `GridChunk` stores its parent grid as a raw `UIGrid*` pointer at `GridChunk.h:39`. This is the same class of bug as `GridLayer::parent_grid` (#270) — the raw pointer can dangle if the grid is destroyed while chunks are still referenced. ## Root Cause `GridChunk.h:39`: ```cpp class GridChunk { // ... UIGrid* parent_grid; // Raw pointer // ... }; ``` And the parallel structure at line 72: ```cpp // Another GridChunk variant (likely for entity layer) UIGrid* parent_grid; // Also raw ``` ## Risk GridChunks are owned by the grid, so under normal operation they don't outlive their parent. However: 1. If any code path takes a pointer/reference to a GridChunk and the grid is destroyed, the `parent_grid` pointer dangles 2. The inconsistency between raw pointers (GridChunk, GridLayer, UIGridPoint) and shared_ptr patterns used elsewhere makes the codebase harder to reason about ## Fix Same as #270 — convert to `std::weak_ptr<UIGrid>` or document the ownership invariant. Since GridChunks are always owned by grids and never exposed to Python, this is lower priority than the GridLayer and UIGridPoint cases. ## Severity **Low** — GridChunks are internal to UIGrid and not exposed to Python, so they generally can't outlive their parent grid. Fixing this is more about consistency than preventing crashes. ## Related - #270 — GridLayer::parent_grid (same pattern, higher risk because layers are Python-accessible) - #271 — UIGridPoint::parent_grid (same pattern)

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:26 +00:00

john referenced this issue

2026-03-08 03:20:27 +00:00

[Minor Feature] Regression tests that trigger each bug from #258–#278 #287

john added a new dependency

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

#252 GridView and Grid, FOVLayer and PathLayer

john commented

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

Author

Owner

Deferred from the #258–#278 memory safety audit. GridChunk is internal to UIGrid and not Python-accessible, so the risk is low. In #252, the plan states GridChunk.h/cpp gets removed entirely — "GridPoint storage replaced by DiscreteMap slots." The chunk rendering architecture continues for layers but GridChunk's parent_grid raw pointer ceases to exist. Linked as dependency of #252.

Deferred from the #258–#278 memory safety audit. GridChunk is internal to UIGrid and not Python-accessible, so the risk is low. In #252, the plan states GridChunk.h/cpp gets removed entirely — "GridPoint storage replaced by DiscreteMap slots." The chunk rendering architecture continues for layers but GridChunk's `parent_grid` raw pointer ceases to exist. 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:12 +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. Raw UIGrid* in GridChunk will become weak_ptr<Grid> during the architecture overhaul.

## Roadmap context Folded into #252 (Grid/GridView split) in the Grid & Entity Overhaul Roadmap (`docs/GRID_ENTITY_OVERHAUL_ROADMAP.md`), **Phase 4**. Raw `UIGrid*` in GridChunk will become `weak_ptr<Grid>` during the architecture overhaul.

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