USB: Always delete transfers even when force-canceling.

We can't call the callbacks (as the comments correctly indicated),
but we can certainly delete the transfer objects, since they
rightfully belong to us.

Should fix #19242, #19241, #19180 and possibly other recent regressions.
Also fixes a long-standing memory leak from this scenario.
This commit is contained in:
Augustin Cavalier
2024-11-12 21:58:16 -05:00
parent 75a4dfe4a1
commit f8a8e56595
4 changed files with 68 additions and 63 deletions
+14 -13
View File
@@ -1761,18 +1761,13 @@ EHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
descriptor = descriptor->next_log; descriptor = descriptor->next_log;
} }
if (!force) { transfer_entry *entry
// if the transfer is canceled by force, the one causing the = (transfer_entry *)malloc(sizeof(transfer_entry));
// cancel is probably not the one who initiated the transfer if (entry != NULL) {
// and the callback is likely not safe anymore entry->transfer = current->transfer;
transfer_entry *entry current->transfer = NULL;
= (transfer_entry *)malloc(sizeof(transfer_entry)); entry->next = list;
if (entry != NULL) { list = entry;
entry->transfer = current->transfer;
current->transfer = NULL;
entry->next = list;
list = entry;
}
} }
current->canceled = true; current->canceled = true;
@@ -1785,7 +1780,13 @@ EHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
while (list != NULL) { while (list != NULL) {
transfer_entry *next = list->next; transfer_entry *next = list->next;
list->transfer->Finished(B_CANCELED, 0);
// if the transfer is canceled by force, the one causing the
// cancel is possibly not the one who initiated the transfer
// and the callback is likely not safe anymore
if (!force)
list->transfer->Finished(B_CANCELED, 0);
delete list->transfer; delete list->transfer;
free(list); free(list);
list = next; list = next;
+32 -30
View File
@@ -704,38 +704,34 @@ OHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
current->endpoint->head_physical_descriptor current->endpoint->head_physical_descriptor
= current->endpoint->tail_physical_descriptor; = current->endpoint->tail_physical_descriptor;
if (!force) { if (pipe->Type() & USB_OBJECT_ISO_PIPE) {
if (pipe->Type() & USB_OBJECT_ISO_PIPE) { ohci_isochronous_td *descriptor
ohci_isochronous_td *descriptor = (ohci_isochronous_td *)current->first_descriptor;
= (ohci_isochronous_td *)current->first_descriptor; while (descriptor) {
while (descriptor) { uint16 frame = OHCI_ITD_GET_STARTING_FRAME(
uint16 frame = OHCI_ITD_GET_STARTING_FRAME( descriptor->flags);
descriptor->flags); _ReleaseIsochronousBandwidth(frame,
_ReleaseIsochronousBandwidth(frame, OHCI_ITD_GET_FRAME_COUNT(descriptor->flags));
OHCI_ITD_GET_FRAME_COUNT(descriptor->flags)); if (descriptor
if (descriptor == (ohci_isochronous_td*)current->last_descriptor)
== (ohci_isochronous_td*)current->last_descriptor) // this is the last ITD of the transfer
// this is the last ITD of the transfer break;
break;
descriptor descriptor
= (ohci_isochronous_td *) = (ohci_isochronous_td *)
descriptor->next_done_descriptor; descriptor->next_done_descriptor;
}
}
// If the transfer is canceled by force, the one causing the
// cancel is probably not the one who initiated the transfer
// and the callback is likely not safe anymore
transfer_entry *entry
= (transfer_entry *)malloc(sizeof(transfer_entry));
if (entry != NULL) {
entry->transfer = current->transfer;
current->transfer = NULL;
entry->next = list;
list = entry;
} }
} }
transfer_entry *entry
= (transfer_entry *)malloc(sizeof(transfer_entry));
if (entry != NULL) {
entry->transfer = current->transfer;
current->transfer = NULL;
entry->next = list;
list = entry;
}
current->canceled = true; current->canceled = true;
} }
current = current->link; current = current->link;
@@ -745,7 +741,13 @@ OHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
while (list != NULL) { while (list != NULL) {
transfer_entry *next = list->next; transfer_entry *next = list->next;
list->transfer->Finished(B_CANCELED, 0);
// If the transfer is canceled by force, the one causing the
// cancel is possibly not the one who initiated the transfer
// and the callback is likely not safe anymore
if (!force)
list->transfer->Finished(B_CANCELED, 0);
delete list->transfer; delete list->transfer;
free(list); free(list);
list = next; list = next;
+14 -13
View File
@@ -1006,18 +1006,13 @@ UHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
descriptor = (uhci_td *)descriptor->link_log; descriptor = (uhci_td *)descriptor->link_log;
} }
if (!force) { transfer_entry *entry
// if the transfer is canceled by force, the one causing the = (transfer_entry *)malloc(sizeof(transfer_entry));
// cancel is probably not the one who initiated the transfer if (entry != NULL) {
// and the callback is likely not safe anymore entry->transfer = current->transfer;
transfer_entry *entry current->transfer = NULL;
= (transfer_entry *)malloc(sizeof(transfer_entry)); entry->next = list;
if (entry != NULL) { list = entry;
entry->transfer = current->transfer;
current->transfer = NULL;
entry->next = list;
list = entry;
}
} }
current->canceled = true; current->canceled = true;
@@ -1029,7 +1024,13 @@ UHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
while (list != NULL) { while (list != NULL) {
transfer_entry *next = list->next; transfer_entry *next = list->next;
list->transfer->Finished(B_CANCELED, 0);
// if the transfer is canceled by force, the one causing the
// cancel is possibly not the one who initiated the transfer
// and the callback is likely not safe anymore
if (!force)
list->transfer->Finished(B_CANCELED, 0);
delete list->transfer; delete list->transfer;
free(list); free(list);
list = next; list = next;
+8 -7
View File
@@ -1117,12 +1117,8 @@ XHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
if (td->transfer == NULL) if (td->transfer == NULL)
continue; continue;
// We can't cancel or delete transfers under "force", as they probably transfers[transfersCount] = td->transfer;
// are not safe to use anymore. transfersCount++;
if (!force) {
transfers[transfersCount] = td->transfer;
transfersCount++;
}
td->transfer = NULL; td->transfer = NULL;
} }
@@ -1179,7 +1175,12 @@ XHCI::CancelQueuedTransfers(Pipe *pipe, bool force)
endpointLocker.Unlock(); endpointLocker.Unlock();
for (int32 i = 0; i < transfersCount; i++) { for (int32 i = 0; i < transfersCount; i++) {
transfers[i]->Finished(B_CANCELED, 0); // If the transfer is canceled by force, the one causing the
// cancel is possibly not the one who initiated the transfer
// and the callback is likely not safe anymore.
if (!force)
transfers[i]->Finished(B_CANCELED, 0);
delete transfers[i]; delete transfers[i];
} }