Skip to content

Fix Chunk destructor segfault using smart pointers #112#414

Merged
void2012 merged 1 commit intosmartcmd:mainfrom
ModMaker101:fix/chunk-destructor-segfault-112
Mar 4, 2026
Merged

Fix Chunk destructor segfault using smart pointers #112#414
void2012 merged 1 commit intosmartcmd:mainfrom
ModMaker101:fix/chunk-destructor-segfault-112

Conversation

@ModMaker101
Copy link
Contributor

Description

This PR fixes a seg-fault that occurs when exiting the game after loading a world. The exception was triggered inside Chunk::~Chunk due to a double free of the AABB bounding box pointer. The fix migrates the raw AABB* to a shared_ptr<AABB> and removes the manual delete logic.

Changes

Previous Behavior

  • Chunk used a raw pointer
  • bb was allocated via AABB::newPermanent but also freed manually in Chunk::~Chunk.
  • Some engine systems also referenced the same AABB. causing an eventual double-free during shutdown.
  • Exiting a world triggered a crash inside _free_dbg when bb was deleted twice.

Root Cause

The root cause was mismatched ownership and lifetime management of the AABB instance. AABB::newPermanet returns an object expected to be globally persistent or managed elsewhere, but Chunk unconditionally deleted it in its destructor. When the engine`s global destructors ran later, the same memory was freed again causing the seg-fault.

New Behavior

  • Chunk now stores bb using shared_ptr<AABB>.
  • The destructor uses = default removing manual deletion.
  • cull now correctly passes bb.get when a raw pointer is required.

Fix Implementation

  • Replaced AABB* bb with shared_ptr<ABB> bb
  • Updated construction.
  • Removed manual memory management.
  • Adjusted cull.
  • Initialized bb = nullptr in the default constructor.

Related Issues

#112
#380

@ModMaker101
Copy link
Contributor Author

2026-03-04.09-02-08.mp4

Here's proof.

@void2012
Copy link
Collaborator

void2012 commented Mar 4, 2026

Duplicate of #380 ?

@ModMaker101
Copy link
Contributor Author

Yes, but my PR fully replaces all raw pointer usage and removes the manual destructor. To basically provide more consistent RAII ownership. #380 keeps mixed pointer semantics, while mine applies a more unified ownership model throughout the class.

@void2012
Copy link
Collaborator

void2012 commented Mar 4, 2026

Alright, merging this one and closing that

@ModMaker101
Copy link
Contributor Author

Alright, thanks.

@void2012 void2012 merged commit 2be856a into smartcmd:main Mar 4, 2026
PortalG pushed a commit to PortalG/MinecraftConsoles that referenced this pull request Mar 5, 2026
wolverton912 added a commit to wolverton912/MinecraftConsoles that referenced this pull request Mar 5, 2026
* Win64: show actual world names in save list, sort newest-first, preserve level name on load/resave

* Refactor README for consistent formatting

Updated formatting for clarity and consistency in the README.

* Prevent world input from affecting inventory (smartcmd#354)

* fix: properly offset the mouse position in containers (smartcmd#327)

* Fix overlapping debug menus and screens (smartcmd#294)

* Fix overlapping debug menus and screens

Also resolves a formatting issue with clang-format

* Update readme

* Update crafting controls description (smartcmd#359)

Clarified crafting controls in README.

* Disable flight state when riding entities (smartcmd#368)

* renderer: frustum test new chunks so newly generated chunks render to full view distance smartcmd#175 (smartcmd#344)

* Removed private information (smartcmd#333)

Maybe we should not have this out there to find so easily?

* Delete README.md

Unnecessary

* fix: fix inverted pitch in the second third person view

* fix: fix horse texture rendering

* feat: add support for username, IP, and port configuration via launch arguments

* feat: headless server

* docs: mark V-Sync note as WIP in README.md

* docs: fix table formatting in README.md

* Disable automatic Windows account username

This could reveal someone's private information on a livestream or video.
We need a long-term username implementation

* Restore username.txt loading without conflict

Still allows for -name launch argument, but restores old expected behavior

* Update README with info about username.txt

* Enable Stained Glass in Creative Menu

* Fix creative inventory crash with Art Tools debug option (smartcmd#399)

Fix vector out-of-bounds crash when scrolling the potions tab in the creative inventory with Art Tools debug enabled.

- Fix getPageCount() returning total rows instead of scrollable pages in Art Tools mode

- Fix off-by-one boundary check in populateMenu() for both static and debug group loops (< should be <=)

Fixes smartcmd#386

* Fix: Sorted the item list in the debug overlay (smartcmd#340)

* Fix: Sorted the item list in the debug overlay

* revert show all files to false

* Revert ShowAllFiles by removing it

* removed extra line

* Adressed PR review changes

* Replaced push_back with emplace_back

* Removed redundant emplace_back

* Fix Chunk destructor segfault using smart pointers smartcmd#112 (smartcmd#414)

* Fix duplicated stained glass when art tools enabled (smartcmd#426)

* Prevent door sounds from playing twice smartcmd#392 (smartcmd#425)

* Fix incorrect distance comparison return value (smartcmd#432)

* Fix:Prevent horse spawner crash (smartcmd#433)

* refactor: refactor KBM input code

* Fix mob nametag position to match correct height offset smartcmd#422 (smartcmd#440)

* fix: ignore dedicated server properties in normal world startup

* Fix controller paging regression in creative menu

Preserve smooth row-by-row scrolling for mouse wheel input, but restore
full-page movement for controller/menu scroll actions in the creative
inventory.

Commit 3093ca3 changed page indexing to support smooth scrolling, which
caused ACTION_MENU_OTHER_STICK_UP/DOWN to advance by one row instead of
one page. Track whether the scroll action originated from the mouse
wheel and only use single-row steps in that case.

Fixes smartcmd#253

* fix: restore KBM sprint on either Ctrl key

* fix: fix smartcmd#464

* Fix for any aspect ratio in 3D Environments (smartcmd#320)

* Add initial AnyAspectRatio support

* Remove some logic that didn't work

* Remove rogue back slash

* Remove more remnants

* Update UILayer.h

* Update some comments

* Remove WIP UI changes

* Fix diffs

* Remove UI resize call from `UpdateAspectRatio`

* handle merge conflict

* Update to C++ style static cast

* Fix syntax

* Revert "fix: fix smartcmd#464"

This reverts commit 5922824.

* Update ItemInHandRenderer.cpp (smartcmd#418)

* Persist fullscreen setting and start windowed mode maximized (smartcmd#446)

Save fullscreen toggle state to options.txt when F11 is pressed, and restore it on the next launch. When starting in windowed mode, the window now opens maximized instead of at a smaller default size.

Previously, g_isFullscreen was hardcoded to false and never persisted, so the game always started in windowed mode. The window also started at a non-maximized size requiring manual maximization.

Fixes smartcmd#391

* Ship updated, maybe temporary subtitle

People keep going "Oh no I don't like the red WINDOWS text, oh no! I hate it!
The red windows text is the devil, it will be my undoing! Please remove the red
windows text or I shall simply perish!" And for that? Well, I have removed it. Now
you can enjoy this logo made by @wiskerdmage in the style of the existing Edition
logos. (smartcmd#385 (comment))

* Ship revised logo from @wiskerdmage

This one looks a bit nicer so let's use this for now...

* Adjust entity tracking ticks for several types

This fixes the jittery problem.

* fix: Fixed crash when launching at 720p smartcmd#517

Modify `MediaWindows64.arc` to include `skinHud.swf`

---------

Co-authored-by: dtentiion <dtentiongit@gmail.com>
Co-authored-by: Alezito2008 <92759854+Alezito2008@users.noreply.github.com>
Co-authored-by: 4win <4winyt@gmail.com>
Co-authored-by: rtm516 <rtm516@users.noreply.github.com>
Co-authored-by: Mykey <mykey.discord@gmail.com>
Co-authored-by: ModMaker101 <119018978+ModMaker101@users.noreply.github.com>
Co-authored-by: Tygo de Vries <108730722+TygodeVries@users.noreply.github.com>
Co-authored-by: daoge_cmd <3523206925@qq.com>
Co-authored-by: Loki Rautio <lokirautio@gmail.com>
Co-authored-by: Marlian <84173858+MCbabel@users.noreply.github.com>
Co-authored-by: Davi Eler Magalhães <davi.eler.magalhaes@gmail.com>
Co-authored-by: Fayaz Shaikh <61674751+fayaz12g@users.noreply.github.com>
Co-authored-by: LetsGoAway <68365423+letsgoawaydev@users.noreply.github.com>
Co-authored-by: Siobhan 🏳️‍⚧️ <33694155+siobhan-saoirse@users.noreply.github.com>
Co-authored-by: kuwacom <kuwa.com3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants