Fix crash when closing a device from a USB transfer timeout

device/usb/usb_device_handle_usbfs.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "device/usb/usb_device_handle_usbfs.h"
 
 #if defined(OS_ANDROID) && __ANDROID_API__ < 21
 #include <linux/usb_ch9.h>
 #else
 #include <linux/usb/ch9.h>
@@ skipping 405 matching lines @@
   DCHECK(fd.is_valid());
   DCHECK(blocking_task_runner_);
 
   helper_.reset(new FileThreadHelper(std::move(fd), this, task_runner_));
   blocking_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&FileThreadHelper::Start, base::Unretained(helper_.get())));
 }
 
 scoped_refptr<UsbDevice> UsbDeviceHandleUsbfs::GetDevice() const {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   return device_;
 }
 
 void UsbDeviceHandleUsbfs::Close() {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_)
     return;  // Already closed.
 
+  // Cancelling transfers may run or destroy callbacks holding the last
+  // reference to this object so hold a reference for the rest of this method.
+  scoped_refptr<UsbDeviceHandleUsbfs> self(this);
+  for (const auto& transfer : transfers_)
+    CancelTransfer(transfer.get(), UsbTransferStatus::CANCELLED);
+
   // On the |task_runner_| thread check |device_| to see if the handle is
   // closed. On the |blocking_task_runner_| thread check |fd_.is_valid()| to
   // see if the handle is closed.
   device_->HandleClosed(this);
   device_ = nullptr;
 
-  for (const auto& transfer : transfers_)
-    CancelTransfer(transfer.get(), UsbTransferStatus::CANCELLED);
-
   // Releases |helper_|.
   blocking_task_runner_->PostTask(
       FROM_HERE, base::Bind(&UsbDeviceHandleUsbfs::CloseBlocking, this));
 }
 
 void UsbDeviceHandleUsbfs::SetConfiguration(int configuration_value,
                                             const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
     return;
   }
 
   // USBDEVFS_SETCONFIGURATION synchronously issues a SET_CONFIGURATION request
   // to the device so it must be performed on a thread where it is okay to
   // block.
   blocking_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&UsbDeviceHandleUsbfs::FileThreadHelper::SetConfiguration,
                  base::Unretained(helper_.get()), configuration_value,
                  callback));
 }
 
 void UsbDeviceHandleUsbfs::ClaimInterface(int interface_number,
                                           const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
     return;
   }
 
   if (base::ContainsKey(interfaces_, interface_number)) {
     USB_LOG(DEBUG) << "Interface " << interface_number << " already claimed.";
     task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
     return;
   }
 
   // It appears safe to assume that this ioctl will not block.
   int rc = HANDLE_EINTR(ioctl(fd_, USBDEVFS_CLAIMINTERFACE, &interface_number));
   if (rc) {
     USB_PLOG(DEBUG) << "Failed to claim interface " << interface_number;
   } else {
     interfaces_[interface_number].alternate_setting = 0;
     RefreshEndpointInfo();
   }
   task_runner_->PostTask(FROM_HERE, base::Bind(callback, rc == 0));
 }
 
 void UsbDeviceHandleUsbfs::ReleaseInterface(int interface_number,
                                             const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
     return;
   }
 
   // USBDEVFS_RELEASEINTERFACE may issue a SET_INTERFACE request to the
   // device to restore alternate setting 0 so it must be performed on a thread
   // where it is okay to block.
   blocking_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&UsbDeviceHandleUsbfs::FileThreadHelper::ReleaseInterface,
                  base::Unretained(helper_.get()), interface_number, callback));
 }
 
 void UsbDeviceHandleUsbfs::SetInterfaceAlternateSetting(
     int interface_number,
     int alternate_setting,
     const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
     return;
   }
 
   // USBDEVFS_SETINTERFACE is synchronous because it issues a SET_INTERFACE
   // request to the device so it must be performed on a thread where it is okay
   // to block.
   blocking_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&UsbDeviceHandleUsbfs::FileThreadHelper::SetInterface,
                  base::Unretained(helper_.get()), interface_number,
                  alternate_setting, callback));
 }
 
 void UsbDeviceHandleUsbfs::ResetDevice(const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
     return;
   }
 
   // USBDEVFS_RESET is synchronous because it waits for the port to be reset
   // and the device re-enumerated so it must be performed on a thread where it
   // is okay to block.
   blocking_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&UsbDeviceHandleUsbfs::FileThreadHelper::ResetDevice,
                  base::Unretained(helper_.get()), callback));
 }
 
 void UsbDeviceHandleUsbfs::ClearHalt(uint8_t endpoint_address,
                                      const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
     return;
   }
 
   // USBDEVFS_CLEAR_HALT is synchronous because it issues a CLEAR_FEATURE
   // request to the device so it must be performed on a thread where it is okay
   // to block.
   blocking_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&UsbDeviceHandleUsbfs::FileThreadHelper::ClearHalt,
                  base::Unretained(helper_.get()), endpoint_address, callback));
 }
 
 void UsbDeviceHandleUsbfs::ControlTransfer(
     UsbTransferDirection direction,
     UsbControlTransferType request_type,
     UsbControlTransferRecipient recipient,
     uint8_t request,
     uint16_t value,
     uint16_t index,
     scoped_refptr<net::IOBuffer> buffer,
     size_t length,
     unsigned int timeout,
     const TransferCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     task_runner_->PostTask(
         FROM_HERE,
         base::Bind(callback, UsbTransferStatus::DISCONNECT, nullptr, 0));
     return;
   }
 
   std::unique_ptr<Transfer> transfer(new (0)
                                          Transfer(buffer, callback, nullptr));
   transfer->control_transfer_buffer =
@@ skipping 16 matching lines @@
     SetUpTimeoutCallback(transfer.get(), timeout);
     transfers_.push_back(std::move(transfer));
   }
 }
 
 void UsbDeviceHandleUsbfs::IsochronousTransferIn(
     uint8_t endpoint_number,
     const std::vector<uint32_t>& packet_lengths,
     unsigned int timeout,
     const IsochronousTransferCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   uint8_t endpoint_address = USB_DIR_IN | endpoint_number;
   size_t total_length =
       std::accumulate(packet_lengths.begin(), packet_lengths.end(), 0u);
   scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(total_length));
   IsochronousTransferInternal(endpoint_address, buffer, total_length,
                               packet_lengths, timeout, callback);
 }
 
 void UsbDeviceHandleUsbfs::IsochronousTransferOut(
     uint8_t endpoint_number,
     scoped_refptr<net::IOBuffer> buffer,
     const std::vector<uint32_t>& packet_lengths,
     unsigned int timeout,
     const IsochronousTransferCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   uint8_t endpoint_address = USB_DIR_OUT | endpoint_number;
   size_t total_length =
       std::accumulate(packet_lengths.begin(), packet_lengths.end(), 0u);
   IsochronousTransferInternal(endpoint_address, buffer, total_length,
                               packet_lengths, timeout, callback);
 }
 
 void UsbDeviceHandleUsbfs::GenericTransfer(UsbTransferDirection direction,
                                            uint8_t endpoint_number,
                                            scoped_refptr<net::IOBuffer> buffer,
                                            size_t length,
                                            unsigned int timeout,
                                            const TransferCallback& callback) {
   if (task_runner_->BelongsToCurrentThread()) {
     GenericTransferInternal(direction, endpoint_number, buffer, length, timeout,
                             callback, task_runner_);
   } else {
     task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&UsbDeviceHandleUsbfs::GenericTransferInternal, this,
                    direction, endpoint_number, buffer, length, timeout,
                    callback, base::ThreadTaskRunnerHandle::Get()));
   }
 }
 
 const UsbInterfaceDescriptor* UsbDeviceHandleUsbfs::FindInterfaceByEndpoint(
     uint8_t endpoint_address) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   auto it = endpoints_.find(endpoint_address);
   if (it != endpoints_.end())
     return it->second.interface;
   return nullptr;
 }
 
 UsbDeviceHandleUsbfs::~UsbDeviceHandleUsbfs() {
   DCHECK(!device_) << "Handle must be closed before it is destroyed.";
 }
 
 void UsbDeviceHandleUsbfs::ReleaseFileDescriptor() {
   // Calls to this method must be posted to |blocking_task_runner_|.
   helper_->ReleaseFileDescriptor();
   helper_.reset();
 }
 
 void UsbDeviceHandleUsbfs::CloseBlocking() {
   // Calls to this method must be posted to |blocking_task_runner_|.
   helper_.reset();
 }
 
 void UsbDeviceHandleUsbfs::SetConfigurationComplete(
     int configuration_value,
     bool success,
     const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (success && device_) {
     device_->ActiveConfigurationChanged(configuration_value);
     // TODO(reillyg): If all interfaces are unclaimed before a new configuration
     // is set then this will do nothing. Investigate.
     RefreshEndpointInfo();
   }
   callback.Run(success);
 }
 
 void UsbDeviceHandleUsbfs::ReleaseInterfaceComplete(
     int interface_number,
     const ResultCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   auto it = interfaces_.find(interface_number);
   DCHECK(it != interfaces_.end());
   interfaces_.erase(it);
   RefreshEndpointInfo();
   callback.Run(true);
 }
 
 void UsbDeviceHandleUsbfs::IsochronousTransferInternal(
     uint8_t endpoint_address,
     scoped_refptr<net::IOBuffer> buffer,
     size_t total_length,
     const std::vector<uint32_t>& packet_lengths,
     unsigned int timeout,
     const IsochronousTransferCallback& callback) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     ReportIsochronousError(packet_lengths, callback,
                            UsbTransferStatus::DISCONNECT);
     return;
   }
 
   auto it = endpoints_.find(endpoint_address);
   if (it == endpoints_.end()) {
     USB_LOG(USER) << "Endpoint address " << static_cast<int>(endpoint_address)
                   << " is not part of a claimed interface.";
@@ skipping 27 matching lines @@
 }
 
 void UsbDeviceHandleUsbfs::GenericTransferInternal(
     UsbTransferDirection direction,
     uint8_t endpoint_number,
     scoped_refptr<net::IOBuffer> buffer,
     size_t length,
     unsigned int timeout,
     const TransferCallback& callback,
     scoped_refptr<base::SingleThreadTaskRunner> callback_runner) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (!device_) {
     callback_runner->PostTask(
         FROM_HERE,
         base::Bind(callback, UsbTransferStatus::DISCONNECT, nullptr, 0));
     return;
   }
 
   uint8_t endpoint_address =
       ConvertEndpointDirection(direction) | endpoint_number;
   auto it = endpoints_.find(endpoint_address);
@@ skipping 22 matching lines @@
     USB_PLOG(DEBUG) << "Failed to submit transfer";
     callback_runner->PostTask(
         FROM_HERE, base::Bind(callback, ConvertTransferResult(rc), nullptr, 0));
   } else {
     SetUpTimeoutCallback(transfer.get(), timeout);
     transfers_.push_back(std::move(transfer));
   }
 }
 
 void UsbDeviceHandleUsbfs::ReapedUrbs(const std::vector<usbdevfs_urb*>& urbs) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   for (auto* urb : urbs) {
     Transfer* transfer = static_cast<Transfer*>(urb->usercontext);
     DCHECK_EQ(urb, &transfer->urb);
 
     if (transfer->cancelled) {
       transfer->reaped = true;
       if (transfer->discarded)
         RemoveFromTransferList(transfer);
     } else {
       TransferComplete(RemoveFromTransferList(transfer));
     }
   }
 }
 
 void UsbDeviceHandleUsbfs::TransferComplete(
     std::unique_ptr<Transfer> transfer) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (transfer->cancelled)
     return;
 
   // The transfer will soon be freed. Cancel the timeout callback so that the
   // raw pointer it holds to |transfer| is not used.
   transfer->timeout_closure.Cancel();
 
   if (transfer->urb.type == USBDEVFS_URB_TYPE_ISO) {
     std::vector<IsochronousPacket> packets(transfer->urb.number_of_packets);
     for (size_t i = 0; i < packets.size(); ++i) {
@@ skipping 14 matching lines @@
              transfer->control_transfer_buffer->data() + 8,
              transfer->urb.actual_length);
     }
 
     transfer->RunCallback(ConvertTransferResult(-transfer->urb.status),
                           transfer->urb.actual_length);
   }
 }
 
 void UsbDeviceHandleUsbfs::RefreshEndpointInfo() {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   endpoints_.clear();
 
   const UsbConfigDescriptor* config = device_->active_configuration();
   if (!config)
     return;
 
   for (const auto& entry : interfaces_) {
     auto interface_it = std::find_if(
         config->interfaces.begin(), config->interfaces.end(),
         [entry](const UsbInterfaceDescriptor& interface) {
           uint8_t interface_number = entry.first;
           uint8_t alternate_setting = entry.second.alternate_setting;
           return interface.interface_number == interface_number &&
                  interface.alternate_setting == alternate_setting;
         });
     DCHECK(interface_it != config->interfaces.end());
 
     for (const auto& endpoint : interface_it->endpoints) {
       EndpointInfo& info = endpoints_[endpoint.address];
       info.type = endpoint.transfer_type;
       info.interface = &*interface_it;
     }
   }
 }
 
 void UsbDeviceHandleUsbfs::ReportIsochronousError(
     const std::vector<uint32_t>& packet_lengths,
     const UsbDeviceHandle::IsochronousTransferCallback& callback,
     UsbTransferStatus status) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   std::vector<UsbDeviceHandle::IsochronousPacket> packets(
       packet_lengths.size());
   for (size_t i = 0; i < packet_lengths.size(); ++i) {
     packets[i].length = packet_lengths[i];
     packets[i].transferred_length = 0;
     packets[i].status = status;
   }
   task_runner_->PostTask(FROM_HERE, base::Bind(callback, nullptr, packets));
 }
 
 void UsbDeviceHandleUsbfs::SetUpTimeoutCallback(Transfer* transfer,
                                                 unsigned int timeout) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   if (timeout > 0) {

[mcasas, 2017/04/24 21:19:23]

nit: (timeout is unsigned int).
if (timeout == 0)
  return;
// the rest.

[Reilly Grant (use Gerrit), 2017/04/24 22:14:00]

Done.

     transfer->timeout_closure.Reset(
-        base::Bind(&UsbDeviceHandleUsbfs::CancelTransfer, this, transfer,
-                   UsbTransferStatus::TIMEOUT));
+        base::Bind(&UsbDeviceHandleUsbfs::OnTimeout, this, transfer));
     task_runner_->PostDelayedTask(FROM_HERE,
                                   transfer->timeout_closure.callback(),
                                   base::TimeDelta::FromMilliseconds(timeout));
   }
 }
 
+void UsbDeviceHandleUsbfs::OnTimeout(Transfer* transfer) {
+  CancelTransfer(transfer, UsbTransferStatus::TIMEOUT);
+}
+
 std::unique_ptr<UsbDeviceHandleUsbfs::Transfer>
 UsbDeviceHandleUsbfs::RemoveFromTransferList(Transfer* transfer_ptr) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   auto it = std::find_if(
       transfers_.begin(), transfers_.end(),
       [transfer_ptr](const std::unique_ptr<Transfer>& transfer) -> bool {
         return transfer.get() == transfer_ptr;
       });
   DCHECK(it != transfers_.end());
   std::unique_ptr<Transfer> transfer = std::move(*it);
   transfers_.erase(it);
   return transfer;
 }
 
 void UsbDeviceHandleUsbfs::CancelTransfer(Transfer* transfer,
                                           UsbTransferStatus status) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
+  DCHECK(device_);
+
   if (transfer->cancelled)
     return;
 
   // |transfer| must stay in |transfers_| as it is still being processed by the
   // kernel and will be reaped later.
   transfer->cancelled = true;
+
+  blocking_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&UsbDeviceHandleUsbfs::FileThreadHelper::DiscardUrb,
+                            base::Unretained(helper_.get()), transfer));
+
+  // Cancelling |timeout_closure| and running completion callbacks may free
+  // |this| so these operations must be performed at the end of this function.
   transfer->timeout_closure.Cancel();
 
   if (transfer->urb.type == USBDEVFS_URB_TYPE_ISO) {
     std::vector<IsochronousPacket> packets(transfer->urb.number_of_packets);
     for (size_t i = 0; i < packets.size(); ++i) {
       packets[i].length = transfer->urb.iso_frame_desc[i].length;
       packets[i].transferred_length = 0;
       packets[i].status = status;
     }
     transfer->RunIsochronousCallback(packets);
   } else {
     transfer->RunCallback(status, 0);
   }
-
-  blocking_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&UsbDeviceHandleUsbfs::FileThreadHelper::DiscardUrb,
-                            base::Unretained(helper_.get()), transfer));
 }
 
-
 void UsbDeviceHandleUsbfs::UrbDiscarded(Transfer* transfer) {
+  DCHECK(sequence_checker_.CalledOnValidSequence());
   transfer->discarded = true;
   if (transfer->reaped)
     RemoveFromTransferList(transfer);
 }
 
 }  // namespace device