Make the daily download limit work with a concurrent queue

The opt-in daily download limit landed after this branch was written and
lives inside the sequential while (Queue.MoveNext()) loop this change
replaces. It is sequential by construction, so rebasing alone leaves it
subtly wrong rather than merely conflicted.

The gate now runs in the dispatch loop, between taking a book off the
queue and starting its task. That keeps the existing semantics - checked
as a book is about to start so the queue keeps its contents and the
limit can be changed mid-run - while books already in flight carry on.
Putting it inside the book task instead would have every blocked book
polling the history at once.

Three concrete defects that fell out of the collision:

RequeueLast deferred a book with Queue.ClearCurrent(), which drops
Active[0]. With one book at a time that is the book being deferred; with
three in flight it is somebody else's download, so deferring the second
active book silently evicted the first. It now removes the book it was
given.

A book cancelled at the gate was left on the active list. The sequential
loop retired it on the next MoveNext(); there is no next MoveNext(), so
it is marked completed explicitly.

CancelAllAsync existed twice after the rebase - the sequential version
that sets cancelAllRequested and cancels Queue.Current, and this
branch's version that cancels every active book. Unified into one that
does both. The flag matters: a queue paused on the limit is sitting in
WaitForDailyLimitAsync and that flag is how it learns to stop.

AnyOtherQueuedBookAllowed enumerates the queue with Queue.Any(...) while
book tasks mutate it; that is safe now that GetAllItems snapshots under
the lock, and it also now takes a copy of the active list to cancel.
This commit is contained in:
Allamagoosa committed 2026-08-16 14:12:00 -07:00
1 parent e49f410544
commit ac19e9e80b
1 file changed
+45 -14
@@ -133,18 +133,6 @@ public class ProcessQueueViewModel : ReactiveObject
private void AddQueueLogEntry(string logMessage)
=> Invoke(() => LogEntries.Add(new(DateTime.Now, logMessage.Trim())));
/// <summary>
/// Clears the queue and cancels the book being processed. Also ends a pause on the daily download limit,
/// which is why both UIs call this instead of manipulating the queue directly.
/// </summary>
public async Task CancelAllAsync()
{
cancelAllRequested = true;
Queue.ClearQueue();
if (Queue.Current is ProcessBookViewModel current)
await current.CancelAsync();
}
#region Add Books to Queue
public async Task<bool> QueueDownloadPdfAsync(IList<LibraryBook> libraryBooks, Configuration? config = null)
@@ -528,7 +516,10 @@ public class ProcessQueueViewModel : ReactiveObject
/// <summary>Moves the book being held back to the end of the queue without counting it as completed.</summary>
private void RequeueLast(ProcessBookViewModel book)
{
Queue.ClearCurrent();
// Removes this book, not the first active one. ClearCurrent() drops Active[0], which with
// several books in flight is some other book's download: deferring the second of three
// active books would silently evict the first instead.
Queue.RemoveActive(book);
Queue.Enqueue([book]);
}
@@ -578,12 +569,23 @@ public class ProcessQueueViewModel : ReactiveObject
/// A book that has already finished processing and so does not need cancelling - typically
/// the one whose result triggered the abort.
/// </param>
/// <summary>
/// Clears the queue and cancels every book currently downloading. Also ends a pause on the daily
/// download limit, which is why both UIs call this instead of manipulating the queue directly.
/// </summary>
/// <param name="except">
/// A book calling this from its own completion path (abort, disk full). It has already finished
/// and must not be asked to cancel itself.
/// </param>
public async Task CancelAllAsync(ProcessBookViewModel? except = null)
{
// Still set here, not only in the sequential path this replaced: a queue paused on the daily
// download limit is waiting inside WaitForDailyLimitAsync and this flag is how it learns to stop.
cancelAllRequested = true;
Queue.ClearQueue();
// Snapshot before cancelling: Active is mutated as each book unwinds.
var inFlight = Queue.Active.Where(b => b != except).ToArray();
var inFlight = Queue.GetActive().Where(b => b != except).ToArray();
await Task.WhenAll(inFlight.Select(b => b.CancelAsync()));
}
@@ -608,6 +610,9 @@ public class ProcessQueueViewModel : ReactiveObject
var _resultLock = new object();
using var abortCts = new CancellationTokenSource();
var activeTasks = new HashSet<Task>();
// Bounds the daily-limit deferral rotation, so a book can never be shuffled to the back
// forever. Counted since the last book that actually started, not since the run began.
int consecutiveDeferrals = 0;
using var counterTimer = new Timer(_ => RunningTime = timeToStr(DateTime.Now - startingTime), null, 0, 500);
@@ -707,6 +712,32 @@ public class ProcessQueueViewModel : ReactiveObject
if (Queue.TryDequeueNext(out var nextBook))
{
// Checked as a book is about to start rather than at queueing time, so the queue keeps
// its contents and the user can raise or turn off the limit mid-run. It belongs in this
// single dispatch loop and not inside the book task: the gate decides whether a book may
// start at all, and a per-book wait would have every blocked book polling at once.
// Books already in flight keep running while the loop is held here.
var gate = await WaitForDailyLimitAsync(nextBook, consecutiveDeferrals);
if (gate is DailyLimitGate.Defer)
{
consecutiveDeferrals++;
RequeueLast(nextBook);
continue;
}
if (gate is DailyLimitGate.Cancelled)
{
Serilog.Log.Logger.Information("Queue was cancelled while waiting on the daily download limit.");
nextBook.Result = ProcessBookResult.Cancelled;
nextBook.Status = ProcessBookStatus.Cancelled;
// The sequential loop left this to the next MoveNext(). Nothing moves this book
// off the active list now, so it is retired here.
Queue.MarkCompleted(nextBook);
continue;
}
consecutiveDeferrals = 0;
activeTasks.Add(ProcessBookAsync(nextBook));
continue;
}