diff --git a/Source/LibationAvalonia/Views/ProcessQueueControl.axaml.cs b/Source/LibationAvalonia/Views/ProcessQueueControl.axaml.cs index d6f872d9..7969640d 100644 --- a/Source/LibationAvalonia/Views/ProcessQueueControl.axaml.cs +++ b/Source/LibationAvalonia/Views/ProcessQueueControl.axaml.cs @@ -79,13 +79,13 @@ public partial class ProcessQueueControl : UserControl }; vm.Queue.Enqueue(testList); - vm.Queue.MoveNext(); - vm.Queue.MoveNext(); - vm.Queue.MoveNext(); - vm.Queue.MoveNext(); - vm.Queue.MoveNext(); - vm.Queue.MoveNext(); - vm.Queue.MoveNext(); + + // Six completed, one active, one still queued - the mix this preview exists to show. + // Driven through the same calls the queue loop makes, rather than the sequential + // MoveNext() that used to live here for this one caller. + for (int i = 0; i < 6 && vm.Queue.TryDequeueNext(out var finished); i++) + vm.Queue.MarkCompleted(finished); + vm.Queue.TryDequeueNext(out _); return; } #endif diff --git a/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs b/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs index ff3da416..6a2fd885 100644 --- a/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs +++ b/Source/LibationUiBase/ProcessQueue/BadBookSessionContext.cs @@ -1,4 +1,4 @@ -using System.Threading; +using System.Threading; using LibationFileManager; namespace LibationUiBase.ProcessQueue; diff --git a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs index fe2edfd4..7e624253 100644 --- a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs +++ b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs @@ -1,4 +1,4 @@ -using ApplicationServices; +using ApplicationServices; using DataLayer; using FileLiberator; using LibationFileManager; @@ -129,6 +129,14 @@ public class ProcessQueueViewModel : ReactiveObject public decimal SpeedLimitIncrement { get; private set; } private decimal _speedLimit; + /// + /// The sequential loop also re-read this from each book as that book started + /// (SpeedLimit = nextBook.Configuration.DownloadSpeedLimit / 1024m / 1024), which kept the + /// displayed number in step when only one book could be downloading. With several running there is + /// no single book to read it back from, and doing so would have whichever book happened to start + /// last overwrite what the user had just typed. The setter is now the only writer: it stores the + /// value once and pushes it out to every active book. + /// public decimal SpeedLimit { get => _speedLimit; @@ -573,9 +581,9 @@ public class ProcessQueueViewModel : ReactiveObject /// Moves the book being held back to the end of the queue without counting it as completed. private void RequeueLast(ProcessBookViewModel book) { - // 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. + // Removes this book by identity, not whichever happens to be first. With several books in + // flight the first active one is some other book's download, so deferring the second of three + // would silently evict the first instead. Queue.RemoveActive(book); Queue.Enqueue([book]); } @@ -688,6 +696,20 @@ public class ProcessQueueViewModel : ReactiveObject using var counterTimer = new Timer(_ => RunningTime = timeToStr(DateTime.Now - startingTime), null, 0, 500); + // True for the one book that gets to tear the queue down, false for every book that arrives + // after it. Written under the lock and read by the dispatch loop without it, hence the + // volatile write. + bool ClaimAbort() + { + lock (resultLock) + { + if (aborted) + return false; + Volatile.Write(ref aborted, true); + return true; + } + } + async Task ProcessBookAsync(ProcessBookViewModel book) { Serilog.Log.Logger.Information("Begin processing queued item: '{item_LibraryBook}'", book.LibraryBook); @@ -717,26 +739,6 @@ public class ProcessQueueViewModel : ReactiveObject Queue.MarkCompleted(book); if (result == ProcessBookResult.FailedAbort) - { - // Same reasoning: several books can run out of disk at once. - if (tearsDownTheQueue) - await CancelAllAsync(book); - // True for the one book that gets to tear the queue down, false for every book that arrives - // after it. Written under the lock and read by the dispatch loop without it, hence the - // volatile write. - bool ClaimAbort() - { - lock (resultLock) - { - if (aborted) - return false; - Volatile.Write(ref aborted, true); - return true; - } - } - - } - else if (result == ProcessBookResult.DiskFull) { if (tearsDownTheQueue) await CancelAllAsync(book); @@ -747,6 +749,12 @@ public class ProcessQueueViewModel : ReactiveObject book.Result = ProcessBookResult.Cancelled; book.Status = ProcessBookStatus.Cancelled; } + } + else if (result == ProcessBookResult.DiskFull) + { + // Same reasoning: several books can run out of disk at once. + if (tearsDownTheQueue) + await CancelAllAsync(book); bool show; lock (resultLock) { show = !shownDiskFullMessage; shownDiskFullMessage = true; } if (show) @@ -847,8 +855,8 @@ public class ProcessQueueViewModel : ReactiveObject 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. + // The sequential loop retired this book on its next step. A dispatch loop has no + // such step, so nothing else would take it off the active list - it is retired here. Queue.MarkCompleted(nextBook); continue; } @@ -887,6 +895,10 @@ public class ProcessQueueViewModel : ReactiveObject } finally { + // Scoped to the run, not to the drain. A queue parked in WaitForDailyLimitAsync only + // re-reads this every DailyLimitPollInterval, so clearing it when the last cancellation + // settles would let the gate wake up, see false, and resume the book just cancelled. + cancelAllRequested = false; DiskSpaceBackupPreflight.ResetBulkPreflightForQueueRun(); } @@ -895,7 +907,3 @@ public class ProcessQueueViewModel : ReactiveObject : $"{time.TotalHours:F0}:{time:mm\\:ss}"; } } - // Scoped to the run, not to the drain. A queue parked in WaitForDailyLimitAsync only - // re-reads this every DailyLimitPollInterval, so clearing it when the last cancellation - // settles would let the gate wake up, see false, and resume the book just cancelled. - cancelAllRequested = false; diff --git a/Source/LibationUiBase/TrackedQueue[T].cs b/Source/LibationUiBase/TrackedQueue[T].cs index 034bbad1..1677d0e4 100644 --- a/Source/LibationUiBase/TrackedQueue[T].cs +++ b/Source/LibationUiBase/TrackedQueue[T].cs @@ -33,9 +33,6 @@ public class TrackedQueue : IReadOnlyCollection, IList, INotifyCollectionC public event EventHandler? QueuedCountChanged; public event NotifyCollectionChangedEventHandler? CollectionChanged; - /// Returns the first active item for backward compatibility (e.g. speed limit display). - public T? Current => _active.FirstOrDefault(); - public IReadOnlyList Active => _active; public IReadOnlyList Completed => _completed; private List Queued { get; } = new(); @@ -317,7 +314,7 @@ public class TrackedQueue : IReadOnlyCollection, IList, INotifyCollectionC } /// - /// The active items, copied under the lock. is the live list, so enumerating + /// The active items, copied under the lock. The backing list is live, so enumerating /// it while book tasks start and finish throws; callers that need to iterate use this. /// public IReadOnlyList GetActive() @@ -326,22 +323,6 @@ public class TrackedQueue : IReadOnlyCollection, IList, INotifyCollectionC return _active.ToList(); } - /// Legacy single-item sequential accessor — kept for compatibility. - public void ClearCurrent() - { - lock (lockObject) - { - var first = _active.FirstOrDefault(); - if (first != null) - { - int displayIndex = _completed.Count; - _active.Remove(first); - Pend(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, first, displayIndex)); - } - } - DispatchPending(); - } - public void ClearQueue() { lock (lockObject) @@ -389,38 +370,6 @@ public class TrackedQueue : IReadOnlyCollection, IList, INotifyCollectionC DispatchPending(); } - /// - /// Legacy sequential MoveNext — completes the first active item and dequeues the next. - /// Only valid when at most one item is active at a time. - /// - public bool MoveNext() - { - try - { - lock (lockObject) - { - var oldActive = _active.FirstOrDefault(); - if (oldActive != null) - { - _active.Remove(oldActive); - _completed.Add(oldActive); - Pend(_completed.Count); - } - if (Queued.Count == 0) - return false; - var next = Queued[0]; - Queued.RemoveAt(0); - _active.Add(next); - PendQueued(Queued.Count); - return true; - } - } - finally - { - DispatchPending(); - } - } - public void Enqueue(IList item) { lock (lockObject) diff --git a/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs b/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs index 100a01e3..5bbabd93 100644 --- a/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs +++ b/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs @@ -171,9 +171,9 @@ this.panel1.Name = "panel1"; this.panel1.Size = new System.Drawing.Size(390, 52); this.panel1.TabIndex = 2; - // + // // queueSettingsTable - // + // this.queueSettingsTable.ColumnCount = 3; this.queueSettingsTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.AutoSize)); this.queueSettingsTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 70F)); @@ -193,9 +193,9 @@ this.queueSettingsTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Percent, 50F)); this.queueSettingsTable.Size = new System.Drawing.Size(220, 50); this.queueSettingsTable.TabIndex = 9; - // + // // autoScrollChk - // + // this.autoScrollChk.Anchor = System.Windows.Forms.AnchorStyles.Left; this.autoScrollChk.AutoSize = true; this.autoScrollChk.Checked = true; @@ -205,9 +205,9 @@ this.autoScrollChk.TabIndex = 6; this.autoScrollChk.Text = "Auto-scroll"; this.autoScrollChk.UseVisualStyleBackColor = true; - // + // // concurrencyLbl - // + // this.concurrencyLbl.Anchor = System.Windows.Forms.AnchorStyles.Left | System.Windows.Forms.AnchorStyles.Right; this.concurrencyLbl.AutoSize = false; this.concurrencyLbl.Name = "concurrencyLbl"; @@ -215,16 +215,16 @@ this.concurrencyLbl.TextAlign = System.Drawing.ContentAlignment.MiddleRight; this.concurrencyLbl.TabIndex = 7; this.concurrencyLbl.Text = "At once:"; - // + // // concurrencyNum - // + // this.concurrencyNum.Anchor = System.Windows.Forms.AnchorStyles.Left; this.concurrencyNum.Name = "concurrencyNum"; this.concurrencyNum.Size = new System.Drawing.Size(84, 23); this.concurrencyNum.TabIndex = 8; - // + // // label1 - // + // this.label1.Anchor = System.Windows.Forms.AnchorStyles.Left | System.Windows.Forms.AnchorStyles.Right; this.label1.AutoSize = false; this.label1.Name = "label1"; @@ -232,9 +232,9 @@ this.label1.TextAlign = System.Drawing.ContentAlignment.MiddleRight; this.label1.TabIndex = 5; this.label1.Text = "DL Limit:"; - // + // // numericUpDown1 - // + // this.numericUpDown1.Anchor = System.Windows.Forms.AnchorStyles.Left; this.numericUpDown1.DecimalPlaces = 1; this.numericUpDown1.Increment = new decimal(new int[] { @@ -260,9 +260,9 @@ 0, 0}); this.numericUpDown1.ValueChanged += new System.EventHandler(this.numericUpDown1_ValueChanged); - // + // // btnCleanFinished - // + // this.btnCleanFinished.Dock = System.Windows.Forms.DockStyle.Right; this.btnCleanFinished.Location = new System.Drawing.Point(298, 0); this.btnCleanFinished.Name = "btnCleanFinished"; @@ -271,9 +271,9 @@ this.btnCleanFinished.Text = "Clear Finished"; this.btnCleanFinished.UseVisualStyleBackColor = true; this.btnCleanFinished.Click += new System.EventHandler(this.btnClearFinished_Click); - // + // // cancelAllBtn - // + // this.cancelAllBtn.Dock = System.Windows.Forms.DockStyle.Left; this.cancelAllBtn.Location = new System.Drawing.Point(0, 0); this.cancelAllBtn.Name = "cancelAllBtn"; diff --git a/Source/_Tests/LibationUiBase.Tests/TrackedQueueTests.cs b/Source/_Tests/LibationUiBase.Tests/TrackedQueueTests.cs index 905667ea..b3b634a3 100644 --- a/Source/_Tests/LibationUiBase.Tests/TrackedQueueTests.cs +++ b/Source/_Tests/LibationUiBase.Tests/TrackedQueueTests.cs @@ -183,7 +183,8 @@ public class TrackedQueueTests public void deferring_an_active_book_sends_it_to_the_back_and_leaves_the_others_running() { // What the daily download limit does when it holds a book back: the book being deferred is - // removed and re-queued. ClearCurrent() would have dropped A, some other book's download. + // removed by identity and re-queued. Removing whichever was first would have dropped A, some + // other book's download. Book a = new("A"), b = new("B"), c = new("C"); var queue = QueueOf(a, b, c); queue.TryDequeueNext(out _);