Keep the radix sort from recursing forever when it runs out of files

radix1() subdivides a bucket by the next splitbits bits of the index, and
stops recursing once prefix + splitbits reaches the width of the index.
The number of buckets comes from the number of files still available, which
shrinks at every level, so deep enough recursion reaches availfiles / 4 == 1
and therefore splitbits == 0. At that point the recursion consumes no bits
of the index and availfiles stops shrinking, so prefix never advances and
the recursion has no way to terminate.

A splitbits of 0 also makes the shift that chooses a feature's bucket a
shift by the full width of the index, which is undefined. In practice it
leaves the shift count masked to zero, so the bucket number is the whole
index rather than 0, and writing to that bucket runs off the end of the
arrays of open files.

Require at least two buckets so that each subdivision always consumes at
least one bit of the index and the shift is always in range, and don't
recurse at all when the next level would not have enough files to split
with: sort that bucket in memory instead, even though it is larger than
the memory limit asked for, since that is the only way left to get it
sorted.

--prefer-radix-sort, which lowers the memory limit to 8K to exercise this
code, segfaults on tests/feature-filter/in.json without this. The other
inputs that it fails on are failing for an unrelated reason, which is
fixed separately in #402; with both changes, --prefer-radix-sort produces
output identical to the ordinary in-memory sort on all of them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wLk2itWETPBAS9a9yE8zu
This commit is contained in:
Claude
2026-08-05 16:47:21 +00:00
parent 7c80fccdc1
commit 23fe065aea
+18 -3
View File
@@ -742,8 +742,16 @@ void start_parsing(int fd, STREAM *fp, long long offset, long long len, std::ato
}
void radix1(int *geomfds_in, int *indexfds_in, int inputs, int prefix, int splits, long long mem, const char *tmpdir, long long *availfiles, FILE *geomfile, FILE *indexfile, std::atomic<long long> *geompos_out, long long *progress, long long *progress_max, long long *progress_reported, int maxzoom, int basezoom, double droprate, double gamma, struct drop_state *ds) {
// Arranged as bits to facilitate subdividing again if a subdivided file is still huge
int splitbits = log(splits) / log(2);
// Arranged as bits to facilitate subdividing again if a subdivided file is still huge.
//
// There must be at least two buckets: with only one, each subdivision would
// consume no bits of the index, so it would never reach the maximum prefix
// that stops the recursion, and the shift below would be by the full width
// of the index, which is undefined.
int splitbits = 1;
if (splits > 1) {
splitbits = log(splits) / log(2);
}
splits = 1 << splitbits;
FILE *geomfiles[splits];
@@ -890,7 +898,14 @@ void radix1(int *geomfds_in, int *indexfds_in, int inputs, int prefix, int split
}
if (indexst.st_size > 0) {
if (indexst.st_size + geomst.st_size < mem) {
// Subdividing again would only make progress if the next level could
// split into at least two buckets, which it can't if there are no longer
// enough files left to do it with. In that case, sort in memory instead,
// even though this is more memory than we wanted to use at once, since
// it is the only way left to get this bucket sorted.
bool can_subdivide = *availfiles / 4 > 1;
if (indexst.st_size + geomst.st_size < mem || !can_subdivide) {
std::atomic<long long> indexpos(indexst.st_size);
int bytes = sizeof(struct index);