Fix heap-use-after-free in threaded area/radius propagation sweeps (~50% of threaded area runs corrupt, then segfault)

[huxnlk]

Preamble

First, thanks for maintaining this. With Cloud-RF/Signal-Server now
history-only, this fork is the practical home of the ITM/ITWOM tooling, and
the CMake build, the srtm2sdf converters and the antenna-pattern toolchain
all worked out of the box for us.

I have been standing up a VHF/UHF coverage pipeline against commit
7f6242af and hit a data race in the threaded area sweep that silently
corrupts the result about half the time. Full repro, a stack trace, and a
verified fix below.

Environment

Item	Value
Commit	7f6242afb3685ff31d9ad14062b80d692ee56327
Binary	signalserverHD
Build	Ubuntu 24.04, cmake ../src && make, gcc 13.3.0, cmake 3.28.3
Terrain	Copernicus GLO-30 converted with the repo's own srtm2sdf-hd
Model	-pm 1 (ITM)
Tile	one home tile plus a neighbour the search radius reaches into, both 3600 samples/degree (12,960,000 postings)
Banner	Built for 32 DEM tiles at 3600 pixels - confirmed on every run below

A data race in the threaded segment workers silently corrupts the coverage result, then segfaults

Severity: high. Roughly half of all threaded area runs produce a wrong
answer. The wrong answer is written to a complete, well-formed .ppm and
accompanied by a printed summary line, so nothing about the output file itself
says it is bad.

What happens

Seven consecutive runs, byte-identical command line, same tiles, same
container, nothing else changed. The runs split into two clean populations:

Run	Area (km2)	Exit	Expected point count printed	Area boundaries bbox
1	49.849	0	11714	correct
2	49.849	0	11714	correct
3	40.391	139	8786	corrupt
4	40.584	139	8786	corrupt
5	40.820	139	8786	corrupt
6	49.849	0	11714	correct
7	41.258	139	8786	corrupt

The three clean runs agree exactly. A -nothreads serial run of the same
command gives 49.848 km2, matching them to 0.001 km2. So when the race
does not fire, the threaded sweep is correct.

When it does fire, three things happen together:

1. The run prints a wrong expected point total - 8786 instead of 11714,
   consistently ~75% of the correct value, i.e. about three segments' worth
   out of the four in Map segments: 4.
2. The Area boundaries: line is printed with nonsense longitudes:
   -5.910478 | 1619.495330 | -6.719922 | -1308.504670. Valid longitude is
   -180 to 180 and this AOI is ~155-156. Note the fractional parts
   (.495330, .504670) are exactly those of the site longitude, so these
   look like the correct value plus a large garbage integer, not random
   memory.
3. The reported area drops ~19%, to 40.4-41.3 km2.

Then the process exits 139 (SIGSEGV) - after a complete .ppm has been
written (25,622,765 bytes, identical size in all seven run directories,
crashed or not).

Root cause

AddressSanitizer resolves this to a heap-use-after-free at
src/models/los.cc:175, inside rangePropagation:

==ERROR: AddressSanitizer: heap-use-after-free on address 0x51b000000220
READ of size 8 at 0x51b000000220 thread T2
    #0 rangePropagation /src/src/models/los.cc:175
    ... [std::async task machinery] ...

freed by thread T0 here:
    #0 operator delete(...)
    #1 std::allocator<PropagationRange>::deallocate(...)
    ... [std::vector<PropagationRange> deallocation] ...

Reading src/models/los.cc at this commit, PlotPropagation() (and, we
found once we looked, the same pattern in PlotLOSMap() and
PlotPropagationRadius()):

1. Builds a std::vector<PropagationRange> (ranges/r/radii) fully
   before starting any thread.
2. Dispatches one std::async(std::launch::async, rangePropagation, ...)
   per segment - a raw pointer into the vector's own backing array is
   handed to each worker. The returned std::future is stored in
   futures, a std::vector<std::future<void *>> declared at namespace
   scope (not local to the function, and shared across all three call
   sites).
3. rangePropagation() casts the pointer back to PropagationRange *v
   and dereferences it in a loop; line 175 is one such dereference, in the
   loop's own exit condition.
4. After dispatch, PlotPropagation() calls finishProgress() to "wait"
   for the workers. That function polls progress counters
   (incremented inside each worker's loop body) on a 500 ms sleep cycle,
   and returns as soon as the running total matches the expected total. It
   never calls .get(), .wait(), or otherwise touches futures or the
   std::async return values - its own comment says "then finish out any
   running threads", but that step is not implemented.
   (PlotLOSMap() and PlotPropagationRadius() call finishThreads()
   instead, which only joins the separate, always-empty threads vector -
   an equivalent no-op, since nothing is ever pushed into threads.)
5. Because a worker increments its counter before re-checking its own
   loop condition and running its cleanup/return, it can be observed as
   "done" the instant its last counter increment lands - while it still
   has one more dereference of v to make before it actually exits.
6. futures never being read also removes the other safety net: a
   std::future from std::async(std::launch::async, ...) normally
   blocks in its destructor until the thread completes - but only if it is
   destroyed or waited on. Living at namespace scope, futures is never
   destroyed between calls, so that implicit wait never fires either.
7. Once the (incomplete) wait returns, PlotPropagation() erases every
   element of ranges and, at function return, destructs the vector -
   freeing its backing storage. If a worker's exit was still pending, it
   now dereferences freed memory at line 175. That is exactly what ASan
   caught: freed by the main thread, read by a worker thread.

A second, independent defect in the same cleanup step:

for(size_t i = 0; i < ranges.size(); i++){
    ranges.erase(ranges.begin() + i);
}

Erasing index i shifts every later element down and shrinks .size() by
one, but i still advances by one next iteration - so this skips every
other element, and on an even-sized vector its last iteration calls
erase(end()), which is undefined behaviour.

The fix

This PR:

• Makes futures local to each of PlotLOSMap(), PlotPropagation() and
  PlotPropagationRadius() instead of namespace-scope, so it can never be
  shared or left stale across calls.
• Adds an explicit for (auto &f : futures) { f.get(); } after
  finishProgress()/finishThreads() in all three functions, so every
  worker is actually awaited before its backing vector can be touched
  again or destructed.
• Replaces the erase-by-index loop in PlotPropagation() with
  ranges.clear() (every element is being discarded anyway, so this also
  sidesteps the same-class bug rather than just patching the loop bounds).

I deliberately kept the diff to these three call sites and did not touch
finishProgress()/finishThreads() themselves, or the identical
erase-by-index pattern in PlotPropagationRadius()'s radii cleanup
(same file, a few dozen lines further down)

Verification

Built commit 7f6242af plus this patch, Ubuntu 24.04, gcc 13.3.0, exact
repro command below.

• 10/10 consecutive threaded area-sweep runs: exit 0, correct
  Area boundaries line, byte-identical 25,622,765-byte coverage.ppm.
  Before the fix, the same build/command/tiles reproduced the crash in
  roughly half of runs (fresh 7-run series immediately before this fix:
  6 clean, 1 crashed with the corrupt bbox above - matching the original
  table).
• 6/6 runs under AddressSanitizer (-g -fsanitize=address -fno-omit-frame-pointer, ASAN_OPTIONS=detect_leaks=0): zero
  heap-use-after-free errors. The same ASan build without the fix
  reproduces the src/models/los.cc:175 UAF reliably.