From 23fe065aea81dd88c6462368083865cde7e586cd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 16:47:21 +0000 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_011wLk2itWETPBAS9a9yE8zu --- main.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/main.cpp b/main.cpp index 1f8b8f83..16f47200 100644 --- a/main.cpp +++ b/main.cpp @@ -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 *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 indexpos(indexst.st_size); int bytes = sizeof(struct index);