Fix confirmed memory and frontend resource leaks#924
Open
Symmetricity wants to merge 1 commit into
Open
Conversation
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>
83fe454 to
c40a15d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Valgrind reports every 64 KiB
PooledStringslab 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 admittedSHPObjects withoutan 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, whichwould leak and bypass ordinary
std::out_of_rangecatches.Changes
PooledStringslabs withunique_ptrplus afreedeleter,preserving table indexes, stored pointers, and string equality semantics.
std::out_of_rangeby value.FILE*ownership for main config, GeoJSON/JSONL, and newline-chunkparsing, including a checked open with a local error.
SHPObjectunder shared exception-safe ownership compatiblewith the supported Boost/Asio handler requirements.
thread so cleanup runs and the failing external source is identified.
PooledStringcoverage across inline/pooled and maximum-lengthboundaries, multiple slabs, and concurrent allocation.
Validation
make test(C++14): all 13 test targets pass.x64-windows-static-md, static dependencies, parallel 8: both executableslink;
tilemaker.exe --helppasses.git diff --check: passes on Linux and Windows.Matched Monaco Memcheck (
--threads 1, PMTiles, example profile, no store):The generated PMTiles files are byte-identical.
Targeted
--track-fds=yeschecks also go clean for: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
AttributePairTypebranch is not constructible through itstwo-bit field without adding a test-only production hook, so the throw-by-value
change is covered by source review and both compiler modes.