john/McRogueFace

Fork 0

[Bugfix] UIGridPoint::parent_grid is a dangling raw pointer #271

New issue

Closed

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

john commented

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

Owner

Summary

UIGridPoint stores its parent grid as a raw UIGrid* pointer (UIGridPoint.h:44). This pointer is used for TCOD map synchronization when walkable or transparent properties are set. While the PyUIGridPointObject struct holds a shared_ptr<UIGrid> to keep the grid alive, the UIGridPoint itself stores a raw pointer that isn't covered by the same safety.

Root Cause

UIGridPoint.h:44:

class UIGridPoint {
    ...
    UIGrid* parent_grid;         // Parent grid reference for TCOD sync
    ...
};

Used in UIGridPoint.cpp:86-87:

if (self->data->parent_grid && self->data->grid_x >= 0 && self->data->grid_y >= 0) {
    self->data->parent_grid->syncTCODMapCell(self->data->grid_x, self->data->grid_y);
}

The PyUIGridPointObject wrapper (UIGridPoint.h:23-27) stores:

typedef struct {
    PyObject_HEAD
    UIGridPoint* data;
    std::shared_ptr<UIGrid> grid;    // Keeps grid alive — good!
} PyUIGridPointObject;

The shared_ptr<UIGrid> grid on the Python wrapper keeps the grid alive while the GridPoint object exists, which prevents the immediate dangling issue. However, the UIGridPoint struct itself doesn't know about the shared_ptr, and parent_grid is set during grid construction (UIGrid.cpp:115, 128) as a raw pointer.

Risk Assessment

Currently mitigated by the shared_ptr<UIGrid> on PyUIGridPointObject. The grid can't be destroyed while a Python GridPoint object references it.

Fragile because:

The safety depends on always wrapping UIGridPoint access in a PyUIGridPointObject with the shared_ptr set correctly
Any C++ code accessing UIGridPoint directly (without the Python wrapper) has no such protection
If someone creates a GridPoint wrapper without setting the shared_ptr, the raw pointer dangles

Fix

Consider making UIGridPoint::parent_grid a std::weak_ptr<UIGrid> or documenting the invariant that PyUIGridPointObject::grid must always be set correctly.

Severity

Low — currently protected by the Python wrapper's shared_ptr, but the pattern is fragile and error-prone for future development.

## Summary `UIGridPoint` stores its parent grid as a raw `UIGrid*` pointer (`UIGridPoint.h:44`). This pointer is used for TCOD map synchronization when `walkable` or `transparent` properties are set. While the `PyUIGridPointObject` struct holds a `shared_ptr<UIGrid>` to keep the grid alive, the `UIGridPoint` itself stores a raw pointer that isn't covered by the same safety. ## Root Cause `UIGridPoint.h:44`: ```cpp class UIGridPoint { ... UIGrid* parent_grid; // Parent grid reference for TCOD sync ... }; ``` Used in `UIGridPoint.cpp:86-87`: ```cpp if (self->data->parent_grid && self->data->grid_x >= 0 && self->data->grid_y >= 0) { self->data->parent_grid->syncTCODMapCell(self->data->grid_x, self->data->grid_y); } ``` The `PyUIGridPointObject` wrapper (UIGridPoint.h:23-27) stores: ```cpp typedef struct { PyObject_HEAD UIGridPoint* data; std::shared_ptr<UIGrid> grid; // Keeps grid alive — good! } PyUIGridPointObject; ``` The `shared_ptr<UIGrid> grid` on the Python wrapper keeps the grid alive while the GridPoint object exists, which prevents the immediate dangling issue. However, the `UIGridPoint` struct itself doesn't know about the shared_ptr, and `parent_grid` is set during grid construction (`UIGrid.cpp:115, 128`) as a raw pointer. ## Risk Assessment **Currently mitigated** by the `shared_ptr<UIGrid>` on `PyUIGridPointObject`. The grid can't be destroyed while a Python GridPoint object references it. **Fragile** because: 1. The safety depends on always wrapping UIGridPoint access in a PyUIGridPointObject with the shared_ptr set correctly 2. Any C++ code accessing UIGridPoint directly (without the Python wrapper) has no such protection 3. If someone creates a GridPoint wrapper without setting the shared_ptr, the raw pointer dangles ## Fix Consider making `UIGridPoint::parent_grid` a `std::weak_ptr<UIGrid>` or documenting the invariant that `PyUIGridPointObject::grid` must always be set correctly. ## Severity **Low** — currently protected by the Python wrapper's shared_ptr, but the pattern is fragile and error-prone for future development.

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:08 +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:54 +00:00

Author

Owner

Deferred from the #258–#278 memory safety audit. In #252, UIGridPoint becomes a lightweight proxy that reads/writes named DiscreteMap slots on GridMap — the parent_grid raw pointer is eliminated entirely. The Phase 4 work (commit 115e16f) already converted PyUIGridPointObject from raw UIGridPoint* to coordinate-based (grid, x, y) access, which mitigates the Python-facing risk. The remaining C++ UIGridPoint::parent_grid field disappears with the #252 rearchitecture. Linked as dependency of #252.

Deferred from the #258–#278 memory safety audit. In #252, UIGridPoint becomes a lightweight proxy that reads/writes named DiscreteMap slots on GridMap — the `parent_grid` raw pointer is eliminated entirely. The Phase 4 work (commit 115e16f) already converted PyUIGridPointObject from raw `UIGridPoint*` to coordinate-based `(grid, x, y)` access, which mitigates the Python-facing risk. The remaining C++ `UIGridPoint::parent_grid` field disappears with the #252 rearchitecture. 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:10 +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 UIGridPoint 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 UIGridPoint 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