bug: Stop leaves the UI on STOPPING until every in-flight download finishes on its own

[LeyckerS]

Context

Pressing Stop during a run leaves the interface on STOPPING and the button on CLOSING… for a long time — minutes with large files, and in the worst case far longer. Reported from real use with 8 active transfers at ~213 MB/s and 21 files remaining.

It is not a deadlock. It is an unbounded wait with no feedback, and the reason is that stop only prevents new work from starting; it never reaches the downloads already in flight.

The sequence today:

1. stop() sets the flag and the state (moon_engine.py:509-514):

self._stop_flag = True
self._state = "stopping"

2. _browser_worker exits its loop promptly (moon_engine.py:199): while not self._get("_stop_flag"): — so no new URL is picked up. Correct.

3. But each download runs as its own task in all_tasks, and download_file in moon_download.py only ever checks kill_evt (L489), which is the stall kill. No stop signal reaches an in-flight download.

4. moon_engine.py:354-358 then waits for every straggler to finish naturally:

stragglers = [t for t in all_tasks if not t.done()]
if stragglers:
    self.log(f"  ⚠  {len(stragglers)} straggler tasks finishing...", "warn")
    await asyncio.gather(*stragglers, return_exceptions=True)

5. Only after that does _on_done() (L388-392) set the state to done.

So the wait is bounded by whatever the in-flight transfers take. With eight concurrent streams on multi-hundred-megabyte files that is minutes, and the client timeout is aiohttp.ClientTimeout(total=7200, ...) (moon_download.py:84) — two hours for a transfer that hangs without dropping.

From the user's side there is no way to tell the difference between "finishing eight transfers" and "hung".

The mechanism to fix this already exists

download_file can already abort mid-stream: _StallKill is raised from inside the chunk loop (moon_download.py:488, L531) when the stall detector fires, and the partial .tmp is deliberately left in place so the next attempt resumes with a Range header (L434, L443-444). A stop should take the same path.

What to do

1. Give download_file a stop signal. It already accepts kill_evt: asyncio.Event. Add a second event, or pass the engine's stop state in, and check it in the chunk loop next to the existing if kill_evt.is_set(): at L489.
2. Abort promptly and leave the .tmp alone. Do not delete the partial file — resume already depends on it. A stopped transfer should look like a stall kill from the filesystem's point of view.
3. Do not count a stopped transfer as a failure. It did not fail; the user stopped it. Check what _do_dl does with the returned status (moon_engine.py:170-192) and make sure _fail is not incremented and the URL is not written to failed_links.txt.
4. Say what is happening while it happens. stopping should be visibly finite: the log line at L357 already reports the straggler count, but the GUI header only shows the word STOPPING. Surfacing "waiting for N transfers" is enough — the engine already knows the number.

Out of scope: changing the timeouts, changing the stall detector, cancelling the extraction phase differently.

Acceptance criteria

• Pressing Stop mid-download reaches state done in seconds, not minutes
• In-flight transfers abort rather than running to completion
• Partial .tmp files survive, and a later run resumes from them
• A stopped transfer is not counted in fail and is not written to failed_links.txt
• Starting a new run after a stop works, with counters reset
• No orphaned tasks: nothing keeps writing to disk after the state says done
• pytest tests/ -q passes

Notes for the contributor

Windows is helpful but not required. The engine is driveable headlessly — pytest tests/ -q stubs Chrome and the network at the moon_extract boundary, and a test can start a run against stubbed slow transfers, call stop(), and assert the state reaches done within a bounded time and that .tmp files still exist.

This is not a first issue. It touches the interaction between the engine's state machine, task teardown and the download loop, and getting it wrong means either transfers that keep writing after "done" or partial files that stop resuming. Read _do_dl (moon_engine.py:149-192), the straggler block (L354-358) and the chunk loop (moon_download.py:481-535) together before you start.

Related: #59 covers the fail-fast/collect asymmetry in the same teardown path. They can be done independently, but whoever takes this will read the same code.

Comment here before you start.

[LeyckerS]

Still free, still the most valuable thing open, and nobody has claimed it in five days. Adding the piece that makes it a much smaller job than it looks, because I suspect the size is what is putting people off.

The wire already exists. It is simply not connected to the Stop button.

download_file takes a kill_evt and honours it inside the chunk loop, which is exactly where an abort has to happen for the partial file to stay resumable:

# moon_download.py:455
kill_evt: asyncio.Event,
...
# moon_download.py:530, inside the read loop
if kill_evt.is_set():
    raise _StallKill()

_StallKill unwinds cleanly, leaves the .tmp on disk, and the caller already knows how to record it. Resume then picks it up on the next run — that machinery is finished and tested.

The gap is who sets it. moon_engine.py:177 creates one per download:

kill_evt = asyncio.Event()

and the only thing that ever sets it is the stall detector (moon_download.py:573), when a transfer drops below STALL_MIN_MBS. Engine.stop() sets _stop_flag, and _stop_flag is read in two places — the worker's queue loop (moon_engine.py:212) and the retry check (:297). Neither is inside a download. So pressing Stop stops new work being picked up, and every transfer already running continues to completion, which is what leaves the UI sitting on STOPPING.

So the shape of the fix is: give the engine a way to reach the live kill_evts — keep them in a set, or one shared event alongside the per-download one — and have stop() set them. Points to settle, and say which you chose:

• One shared event or a registry of per-download events? Shared is smaller; per-download keeps the stall detector's semantics separate from a user-initiated stop, which matters because kills_so_far feeds the re-extract logic and a Stop should probably not count as a stall kill.
• stop() currently returns after at most 1.5s (fix: close Chrome before engine shutdown returns #122 made it close the browser and wait). Decide whether it also waits for the transfers to unwind, and say so — the log line still reads "finishing the downloads in flight...", which will need to change either way.

tests/test_exit_cleanup.py from #122 is the pattern for testing this without a network — it drives a real Engine with the extraction stubbed and asserts on what happened before stop() returned.

Windows is helpful but not required: the suite stubs Chrome and the network at the moon_extract boundary, so the behaviour is testable anywhere. Labelled good first issue — not because it is trivial, but because the hard part (aborting mid-chunk without corrupting the partial) is already written and this is wiring.

Comment to claim it.

[LeyckerS]

Fixed by #149, merged just now — thanks @Allen58562.

Stop no longer waits for a transfer that has already started. The engine keeps the kill event belonging to each active download in a registry and signals them with loop.call_soon_threadsafe(kill_evt.set), which is what setting an asyncio.Event from the calling thread requires; entries are discarded in a finally: so the registry cannot grow over a long run.

The part worth knowing if you hit this: a stop is now recorded as stopped rather than as a stall kill. That distinction is load-bearing — a stall kill is re-queued by design, so without it the retry path would have quietly undone the stop. The partial .tmp is kept and stays resumable, and failed_links.txt is not written for a run you ended yourself.

In v4.1 when it ships; on main now.