Skip to content

Fix AppendVector random access iterator#917

Open
Symmetricity wants to merge 1 commit into
systemed:masterfrom
Symmetricity:fix/appendvector-random-access
Open

Fix AppendVector random access iterator#917
Symmetricity wants to merge 1 commit into
systemed:masterfrom
Symmetricity:fix/appendvector-random-access

Conversation

@Symmetricity

@Symmetricity Symmetricity commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

This PR was prepared with AI assistance.

AppendVector::Iterator advertises itself as a random access iterator and is
used by tile sorting code through Boost sorting algorithms. Several iterator
operations were implemented by repeatedly converting between chunk/offset and
absolute positions, while other random-access operations expected by sorting
implementations were missing.

This changes the iterator to track a single absolute index, then derive the
underlying chunk and offset only when dereferencing. It also fills out the
random-access operations used by standard/Boost sorting code, including
comparisons, iterator distance, offset addition/subtraction, and indexing.

The existing append_vector test now exercises the random-access operations,
including postfix movement and movement across the internal chunk boundary.

Why

On a current MSVC toolchain, an Austria-sized OpenMapTiles build could crash
while finalizing z6 output tiles, inside sorting of AppendVector iterators.
Diagnostics showed the sorter reaching an invalid iterator position before the
beginning of the vector. Making the iterator arithmetic use one absolute
position avoids inconsistent chunk/offset state and makes the advertised
iterator category match the supported operations.

Crash evidence

This appears to be exposed by newer MSVC/STL behavior rather than by the input
data. Older VS 2022/MSVC builds used locally had not reproduced this crash, but
the Windows artifact from a newer GitHub Actions release run on the fork
(Symmetricity/tilemaker run 27551026617, artifact 7640269070) reproduced
an access violation while generating Austria tiles. The user-visible failure was
0xc0000005 during z6 finalization, around:

  • osm: finalizing z6 tile 2135/4096

Rebuilding the same code shape with the current Windows toolchain
(MSVC 19.51 / VS 18 2026 Build Tools) also reproduced the crash. A diagnostic
build showed the active sort was OutputObjectXY over about 12.7 million
objects:

  • source=osm, type=OutputObjectXY, indexZoom=14, threads=8,
    objects=12680604
  • invalid iterator state: vec=-1, offset=8191, absolute=-1
  • previous iterator state: vec=0, offset=8190, absolute=8190

That points to an iterator arithmetic/contract issue being surfaced by the
newer sort implementation. With this patch, the same MSVC 19.51 build completed
both Austria MBTiles and PMTiles generation.

Testing

  • cmake -S . -B ... -DCMAKE_BUILD_TYPE=RelWithDebInfo
  • cmake --build ... --parallel 8
  • make test_append_vector
  • make test
  • Windows MSVC 19.51 build with the x64-windows-static-md vcpkg triplet
  • Windows Austria OpenMapTiles MBTiles generation: exit 0
  • Windows Austria OpenMapTiles PMTiles generation: exit 0

Performance

Compared an upstream master build against the same commit plus this change,
using the same RelWithDebInfo build shape and OpenMapTiles profile.
The Austria PMTiles runtime/RSS profile used three alternating runs in each
order. Wall time was effectively unchanged between the forward and reverse
orders. A small peak-RSS high-water movement was observed, but heaptrack and
Massif on a smaller fixture showed allocation counts and heap shape unchanged,
so this does not appear to introduce a meaningful allocation regression.

AppendVector advertises a random access iterator and is passed to sorting algorithms that may rely on the full iterator contract. Newer standard library implementations can exercise distance, comparison, offset, indexing, and postfix operations more aggressively, so storing chunk and offset separately leaves room for invalid iterator arithmetic.

Track iterator position as one absolute index and cover the random access operations in the existing append vector test. This keeps the container shape unchanged while making the advertised iterator category match the supported operations.

Co-authored-by: Codex <noreply@openai.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.

1 participant