Skip to content

Fix undefined behavior in string_pool memory management#150

Merged
sean-parent merged 1 commit intomainfrom
fix/string-pool-memory-management
Feb 5, 2026
Merged

Fix undefined behavior in string_pool memory management#150
sean-parent merged 1 commit intomainfrom
fix/string-pool-memory-management

Conversation

@fosterbrereton
Copy link
Member

Summary

Fixed undefined behavior in string_pool_t destructor where memory allocated with ::operator new was being deallocated with delete instead of ::operator delete.

Details

In source/string_pool.cpp:

  • Line 65 allocates memory using ::operator new(pool_size) for raw memory pools
  • Line 46 (old code) was using adobe::for_each(pool_m, adobe::delete_ptr()) which calls delete
  • This mismatch is undefined behavior according to C++ standard

The fix:

  • Replaced the destructor to use explicit loop with ::operator delete
  • Removed unused includes (adobe/algorithm/for_each.hpp and adobe/functional.hpp)

Impact

This fixes a potential memory corruption issue. While the code may have appeared to work in practice, using delete on memory from ::operator new is undefined behavior and could cause issues with certain allocators or debugging tools.

🤖 Generated with Claude Code

The string_pool_t destructor was using delete on memory allocated
with ::operator new, which is undefined behavior. Memory allocated
with ::operator new must be deallocated with ::operator delete.

Changes:
- Replace adobe::for_each(pool_m, adobe::delete_ptr()) with explicit
  loop using ::operator delete
- Remove unused includes: adobe/algorithm/for_each.hpp and
  adobe/functional.hpp

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sean-parent sean-parent merged commit 133ec0d into main Feb 5, 2026
20 of 26 checks passed
@sean-parent sean-parent deleted the fix/string-pool-memory-management branch February 5, 2026 00:58
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