Skip to content

Fix a double free issue when start == goal - #45

Merged
justinhj merged 1 commit into
masterfrom
fix-double-free
Sep 30, 2026
Merged

justinhj merged 1 commit into
masterfrom
fix-double-free

Conversation

@justinhj

Copy link
Copy Markdown
Owner

Summary

Fixes a bug where initiating and completing a search where the start node is already the goal (`start == goal`) causes

a use-after-free and double-free of m_Start.

### Root Cause
1. When `start == goal`, `SearchStep()` succeeds on the first step. Because `n->m_UserState.IsSameState(m_Start-

m_UserState)is true, the backward solution-chaining loop is skipped andm_Start->childremainsnullptr. 2. FreeUnusedNodes()immediately iterates overm_NodeMapand deletes any node where!n->child. Since m_Start-
child == nullptr, m_Startwas erroneously freed as an "unused" node. 3. Subsequent calls toGetSolutionStart()resulted in a use-after-free. 4. WhenFreeSolutionNodes()was called, it executed itselsebranch (specifically designed to clean upm_Start andm_Goalwhenm_Start->child == nullptr), freeing m_Start` a second time.

### Impact
- In debug builds, `m_AllocateNodeCount` drops to `-1`, triggering an assertion failure in `EnsureMemoryFreed()`.
- With `FixedSizeAllocator`, freeing the same block twice causes a self-referential cycle in the intrusive free-list

(pNode->pNext = pNode), silently corrupting the allocator and causing subsequent allocations to alias the same memory.
Because FixedSizeAllocator manages memory within a single monolithic buffer, AddressSanitizer (ASan) does not detect
this corruption in release builds.

### Fix
- In `FreeUnusedNodes()`, skip `m_Start` (`if (n != m_Start && !n->child)`), ensuring lifetime ownership of `m_Start`

remains solely with FreeSolutionNodes().

### Testing
- Added unit test `Start Node Equals Goal Node` in `tests.cpp` verifying successful search completion, solution

traversal, and memory cleanup without assertions or leaks.
- Verified all existing tests and example executables (findpath, 8puzzle, minpathbucharest, and bench) pass.

@justinhj
justinhj merged commit 2cbdab8 into master Sep 30, 2026
1 check passed
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.

1 participant