From 2f4928cfa3342b892a439fdefba913e44eda7c84 Mon Sep 17 00:00:00 2001 From: John McCardle Date: Fri, 10 Apr 2026 01:34:33 -0400 Subject: [PATCH] Null parent_grid pointers in GridData destructor, closes #270, closes #271, closes #277 GridLayer, UIGridPoint, and GridChunk each stored a raw GridData* that could dangle if the grid was destroyed while external shared_ptrs (e.g. Python layer wrappers) still referenced child objects. The destructor now nulls all parent_grid pointers before cleanup. All usage sites already had null guards, so this completes the fix. These were the last three unfixed bugs from the memory safety audit. Co-Authored-By: Claude Opus 4.6 --- src/GridData.cpp | 24 +++ .../issue_270_277_parent_grid_dangle_test.py | 137 ++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 tests/regression/issue_270_277_parent_grid_dangle_test.py diff --git a/src/GridData.cpp b/src/GridData.cpp index e2aaf45..8829723 100644 --- a/src/GridData.cpp +++ b/src/GridData.cpp @@ -12,6 +12,30 @@ GridData::GridData() GridData::~GridData() { + // #270: Null out parent_grid in all layers so surviving shared_ptrs + // (held by Python wrappers) don't dangle after grid destruction + for (auto& layer : layers) { + if (layer) layer->parent_grid = nullptr; + } + + // #271: Null out parent_grid in all grid points (flat storage) + for (auto& p : points) { + p.parent_grid = nullptr; + } + + // #277: Null out parent_grid in chunks and chunk manager + if (chunk_manager) { + for (auto& chunk : chunk_manager->chunks) { + if (chunk) { + chunk->parent_grid = nullptr; + for (auto& cell : chunk->cells) { + cell.parent_grid = nullptr; + } + } + } + chunk_manager->parent_grid = nullptr; + } + cleanupTCOD(); } diff --git a/tests/regression/issue_270_277_parent_grid_dangle_test.py b/tests/regression/issue_270_277_parent_grid_dangle_test.py new file mode 100644 index 0000000..66ddd8d --- /dev/null +++ b/tests/regression/issue_270_277_parent_grid_dangle_test.py @@ -0,0 +1,137 @@ +"""Regression test: parent_grid dangling raw pointer (#270, #271, #277). + +Bug: GridLayer, UIGridPoint, and GridChunk each stored a raw GridData* +parent_grid pointer. If the grid was destroyed while a C++ shared_ptr +still held a layer, the raw pointer would dangle and subsequent layer +operations that need the grid (like updatePerspective/drawFOV) could crash. + +Fix: GridData::~GridData() now nulls parent_grid in all layers, grid +points, and chunks before destruction. All usage sites already have +null checks so they degrade gracefully. +""" +import mcrfpy +import sys +import gc + +def test_layer_detach_then_access(): + """Layer operations work after explicit detach from grid (#270)""" + grid = mcrfpy.Grid(grid_size=(10, 10)) + layer = mcrfpy.ColorLayer(z_index=-1, name="fog") + grid.add_layer(layer) + layer.fill(mcrfpy.Color(50, 50, 50)) + + # Detach layer - this nulls parent_grid + layer.grid = None + gc.collect() + + # Layer should still be usable for non-grid operations + assert layer is not None + assert layer.name == "fog" + assert layer.z_index == -1 + assert layer.grid is None + print(" PASS: layer_detach_then_access") + +def test_tilelayer_detach_then_access(): + """TileLayer operations work after explicit detach (#270)""" + grid = mcrfpy.Grid(grid_size=(10, 10)) + layer = mcrfpy.TileLayer(z_index=-2, name="terrain") + grid.add_layer(layer) + + layer.grid = None + gc.collect() + + assert layer is not None + assert layer.name == "terrain" + assert layer.z_index == -2 + assert layer.grid is None + print(" PASS: tilelayer_detach_then_access") + +def test_gridpoint_property_setter_with_valid_grid(): + """GridPoint walkable/transparent setters sync TCOD map (#271)""" + grid = mcrfpy.Grid(grid_size=(10, 10)) + gp = grid.at(5, 5) + gp.walkable = True + assert gp.walkable == True + gp.transparent = True + assert gp.transparent == True + + # Verify sync worked by checking the reverse + gp.walkable = False + assert gp.walkable == False + print(" PASS: gridpoint_property_setter_with_valid_grid") + +def test_layer_reattach_to_new_grid(): + """Layer can be detached from one grid and attached to another""" + grid1 = mcrfpy.Grid(grid_size=(10, 10)) + layer = mcrfpy.ColorLayer(z_index=-1, name="fog") + grid1.add_layer(layer) + layer.fill(mcrfpy.Color(50, 50, 50)) + + # Detach from first grid + layer.grid = None + assert layer.grid is None + + # Attach to new grid (same size since layer size must match) + grid2 = mcrfpy.Grid(grid_size=(10, 10)) + grid2.add_layer(layer) + assert layer.grid is not None + print(" PASS: layer_reattach_to_new_grid") + +def test_multiple_layers_detach(): + """Multiple layers can be detached independently""" + grid = mcrfpy.Grid(grid_size=(10, 10)) + color = mcrfpy.ColorLayer(z_index=-1, name="color") + tile = mcrfpy.TileLayer(z_index=-2, name="tile") + grid.add_layer(color) + grid.add_layer(tile) + + color.grid = None + assert color.grid is None + assert tile.grid is not None + + tile.grid = None + assert tile.grid is None + print(" PASS: multiple_layers_detach") + +def test_layer_from_grid_layers_tuple(): + """Layers obtained from grid.layers have proper grid reference""" + grid = mcrfpy.Grid(grid_size=(10, 10)) + layer = mcrfpy.ColorLayer(z_index=-1, name="fog") + grid.add_layer(layer) + + # Get layer through grid.layers property (grid has default 'tilesprite' layer too) + layers = grid.layers + assert len(layers) >= 2 # default tilesprite + our fog + # Find our layer by name + retrieved = [l for l in layers if l.name == "fog"] + assert len(retrieved) == 1 + assert retrieved[0].grid is not None + print(" PASS: layer_from_grid_layers_tuple") + +def test_large_grid_chunks(): + """Large grids use chunk storage; GridPoints have valid parent_grid (#277)""" + # Threshold is 64, so 100x100 uses chunks + grid = mcrfpy.Grid(grid_size=(100, 100)) + gp = grid.at(50, 50) + gp.walkable = True + assert gp.walkable == True + gp.transparent = True + assert gp.transparent == True + + # Edge cell + gp2 = grid.at(99, 99) + gp2.walkable = True + assert gp2.walkable == True + print(" PASS: large_grid_chunks") + + +print("Testing parent_grid dangling pointer fixes (#270, #271, #277)...") +test_layer_detach_then_access() +test_tilelayer_detach_then_access() +test_gridpoint_property_setter_with_valid_grid() +test_layer_reattach_to_new_grid() +test_multiple_layers_detach() +test_layer_from_grid_layers_tuple() +test_large_grid_chunks() +print("PASS: all parent_grid dangling pointer tests passed") +sys.exit(0)