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.
This commit is contained in:
krish rathi 2026-07-30 22:44:57 +05:30 committed by GitHub
parent 6c8499a603
commit 6414a59851
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 48 additions and 2 deletions

View File

@ -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,

View File

@ -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()