From 6414a59851a1be266a9a637e2d5019fd8704f211 Mon Sep 17 00:00:00 2001 From: krish rathi <148011352+krishrathi1@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:44:57 +0530 Subject: [PATCH] fix(ble): don't spend the fragment scheduler's slot budget on blocked requests (#1530) reservePendingStarts() decremented availableSlots for every dequeued pending transfer before checking whether it would actually be admitted. A request blocked because its transferId is already active (a resend of in-flight content sitting at the front of the queue) still consumed a slot even though it was deferred back into the queue rather than started -- so a single blocked front-of-queue item could zero out the budget and end the loop before ever reaching a later, unrelated, genuinely startable pending transfer. That transfer then sat starved until some other transfer happened to complete and trigger another pass, rather than starting immediately when real capacity was already available. Move the decrement to the point where a transfer is actually admitted into activeTransfers, so only genuine starts spend the budget. --- ...BLEOutboundFragmentTransferScheduler.swift | 8 +++- ...tboundFragmentTransferSchedulerTests.swift | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift b/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift index 722e55c4..5176149a 100644 --- a/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift +++ b/bitchat/Services/BLE/BLEOutboundFragmentTransferScheduler.swift @@ -261,8 +261,6 @@ struct BLEOutboundFragmentTransferScheduler { continue } - availableSlots -= 1 - guard activeTransfers.count < maxConcurrentTransfers else { pendingTransfers.insert(request, at: 0) results.append(.queued(request: request, transferId: transferId, position: .front)) @@ -270,11 +268,17 @@ struct BLEOutboundFragmentTransferScheduler { } guard activeTransfers[transferId] == nil else { + // Blocked on an already-active copy of this content: leave + // the slot budget untouched so a later, unrelated pending + // transfer can still start in this same pass instead of + // being starved until some other transfer happens to + // complete. blockedFront.append(request) results.append(.queued(request: request, transferId: transferId, position: .front)) continue } + availableSlots -= 1 activeTransfers[transferId] = ActiveTransferState( totalFragments: 0, sentFragments: 0, diff --git a/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift b/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift index 8ceb1ac9..27670408 100644 --- a/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift +++ b/bitchatTests/Services/BLEOutboundFragmentTransferSchedulerTests.swift @@ -260,6 +260,48 @@ struct BLEOutboundFragmentTransferSchedulerTests { } } + @Test + func blockedDuplicateAtFrontOfQueueDoesNotStarveALaterUnrelatedPendingTransfer() { + // Bug: reservePendingStarts spent the slot budget on a pending + // request the moment it was dequeued, before checking whether that + // request would actually be admitted. A resend of still-active + // content sitting at the front of the queue therefore consumed a + // slot even though it was deferred back to the queue rather than + // started -- starving an unrelated, genuinely startable transfer + // right behind it until some other transfer happened to complete. + var scheduler = BLEOutboundFragmentTransferScheduler() + let t1 = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "t1", payload: "file-a") + let t2 = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "t2", payload: "file-b") + let dupT1 = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "t1", payload: "file-a") + let unrelated = makeRequest(type: MessageType.fileTransfer.rawValue, transferId: "t3", payload: "file-c") + + _ = scheduler.submit(t1, maxConcurrentTransfers: 2) + _ = scheduler.submit(t2, maxConcurrentTransfers: 2) + #expect(scheduler.activeCount == 2) + + // Both slots are full, so a resend of "t1" (still active) and an + // unrelated transfer both land in the pending queue, in that order. + _ = scheduler.submit(dupT1, maxConcurrentTransfers: 2) + _ = scheduler.submit(unrelated, maxConcurrentTransfers: 2) + #expect(scheduler.pendingCount == 2) + + // "t2" finishes; "t1" stays active, so the queued "t1" resend at the + // front of the queue is still blocked when we reserve pending starts. + let didActivate = scheduler.activateReservedTransfer(id: "t2", totalFragments: 1, workItems: []) + #expect(didActivate) + #expect(scheduler.markFragmentSent(transferId: "t2") == .complete(sentFragments: 1, totalFragments: 1)) + + let starts = scheduler.reservePendingStarts(maxConcurrentTransfers: 2) + + let startedTransferIds: [String] = starts.compactMap { + if case let .start(_, reservedTransferId) = $0 { return reservedTransferId } + return nil + } + #expect(startedTransferIds == ["t3"], "the unrelated pending transfer must start in the same pass despite the blocked front item") + #expect(scheduler.activeCount == 2, "t1 (still running) and the newly-started t3") + #expect(scheduler.pendingCount == 1, "only the blocked t1 resend remains queued") + } + @Test func removeAllReturnsActiveWorkItemsAndDropsPendingTransfers() { var scheduler = BLEOutboundFragmentTransferScheduler()