Skip to content

Fix confirmed memory and frontend resource leaks#924

Open
Symmetricity wants to merge 1 commit into
systemed:masterfrom
Symmetricity:fix/confirmed-memory-leaks
Open

Fix confirmed memory and frontend resource leaks#924
Symmetricity wants to merge 1 commit into
systemed:masterfrom
Symmetricity:fix/confirmed-memory-leaks

Conversation

@Symmetricity

Copy link
Copy Markdown
Contributor

Problem

Valgrind reports every 64 KiB PooledString slab as definitely lost at exit.
The sampled Monaco run loses 196,608 bytes in three blocks. Several frontend
error paths also leave FILE*, SHP/DBF handles, or admitted SHPObjects without
an exception-safe owner. Worker exceptions can escape Asio handlers and call
terminate, preventing scoped cleanup and obscuring the source of the error.

AttributePair::hash() also throws a heap-allocated exception pointer, which
would leak and bypass ordinary std::out_of_range catches.

Changes

  • Own stable PooledString slabs with unique_ptr plus a free deleter,
    preserving table indexes, stored pointers, and string equality semantics.
  • Throw std::out_of_range by value.
  • Add scoped FILE* ownership for main config, GeoJSON/JSONL, and newline-chunk
    parsing, including a checked open with a local error.
  • Own SHP and DBF handles independently so half-open pairs are cleaned up.
  • Keep each posted SHPObject under shared exception-safe ownership compatible
    with the supported Boost/Asio handler requirements.
  • Capture worker exceptions, join the pool, and report them from the main
    thread so cleanup runs and the failing external source is identified.
  • Extend PooledString coverage across inline/pooled and maximum-length
    boundaries, multiple slabs, and concurrent allocation.

Validation

  • make test (C++14): all 13 test targets pass.
  • CMake RelWithDebInfo (C++17), parallel 8: passes.
  • Windows VS 2022/MSVC 19.44 RelWithDebInfo with vcpkg
    x64-windows-static-md, static dependencies, parallel 8: both executables
    link; tilemaker.exe --help passes.
  • git diff --check: passes on Linux and Windows.

Matched Monaco Memcheck (--threads 1, PMTiles, example profile, no store):

Upstream This branch
Allocations / frees 563,412 / 563,409 563,407 / 563,407
Bytes in use at exit 196,608 0
Definitely lost 196,608 B / 3 blocks 0
Errors 3 0

The generated PMTiles files are byte-identical.

Targeted --track-fds=yes checks also go clean for:

  • invalid main config (previously one open descriptor and 472 reachable bytes);
  • invalid GeoJSON (previously aborting with the source descriptor open);
  • an SHP/DBF half-pair (previously one open descriptor, 248 definitely lost
    bytes, and 1,272 indirectly lost bytes).

Scope

This intentionally does not reuse partially filled TLS string tails, bound SHP
or output queues, change mmap arenas, or alter geometry/cache storage. Those are
separate peak-memory changes with different concurrency and performance risks.
The direct invalid AttributePairType branch is not constructible through its
two-bit field without adding a test-only production hook, so the throw-by-value
change is covered by source review and both compiler modes.

PooledString allocations remain valid for the process lifetime, but their raw 64 KiB slabs have no owner, so Valgrind reports them as definitely lost. AttributePair::hash() also throws a heap-allocated exception pointer that bypasses ordinary catches.

Give pooled slabs stable RAII ownership and throw hash errors by value. Add scoped ownership for config and GeoJSON FILE handles, partial SHP/DBF opens, and posted SHPObject instances, then propagate worker exceptions to the main thread so those owners unwind and failures include their external source.

Extend pooled-string and file-open tests across boundary, multi-table, and concurrent cases. The generated Monaco archive remains byte-identical while matched Memcheck runs free every allocation and close the targeted error-path descriptors.

Co-authored-by: Codex <noreply@openai.com>
@Symmetricity Symmetricity force-pushed the fix/confirmed-memory-leaks branch from 83fe454 to c40a15d Compare July 11, 2026 09:56
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