Reland "Attach UsbContext to libusb_device(_handle) pointers"

This is a reland of daa348fa

Original change's description:
> Attach UsbContext to libusb_device(_handle) pointers
> 
> This change modifies ScopedLibusbDeviceRef and adds a new class
> ScopedLibusbDeviceHandle. This now explicitly own a reference to the
> UsbContext object (which reference counts a libusb_context) in addition
> to the libusb_device or libusb_device_handle they are wrapping.
> 
> This resolves potential use-after-frees possible when posting tasks
> with a ScopedLibusbDeviceRef since the UsbService the task is being
> posted to could be destroyed before the task is executed. The
> libusb_device would then be released after its libusb_context has been
> destroyed.
> 
> This is based on https://crrev.com/c/1131949 after I realized there
> where additional issues that needed to be addressed.
> 
> Bug: 838947
> Change-Id: Idee02828bf615bd477033e585fffe03cf4d20595
> Reviewed-on: https://chromium-review.googlesource.com/1145910
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Commit-Queue: Reilly Grant <reillyg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577329}

Bug: 838947
Change-Id: Ibb916b6dea86f05f0b6d2fdfa2d3b71f8c1e8c66
Reviewed-on: https://chromium-review.googlesource.com/1158770
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581997}

device/usb/BUILD.gn

@@ -81,6 +81,9 @@ static_library("usb") {
 
   if (is_win || is_mac) {
     sources += [
+      "scoped_libusb_device_handle.cc",
+      "scoped_libusb_device_handle.h",
+      "scoped_libusb_device_ref.cc",
       "scoped_libusb_device_ref.h",
       "usb_context.cc",
       "usb_context.h",

device/usb/scoped_libusb_device_handle.cc

+// Copyright 2018 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/scoped_libusb_device_handle.h"
+
+#include "device/usb/usb_context.h"
+#include "third_party/libusb/src/libusb/libusb.h"
+
+namespace device {
+
+ScopedLibusbDeviceHandle::ScopedLibusbDeviceHandle(
+    libusb_device_handle* handle,
+    scoped_refptr<UsbContext> context)
+    : handle_(handle), context_(std::move(context)) {}
+
+ScopedLibusbDeviceHandle::ScopedLibusbDeviceHandle(
+    ScopedLibusbDeviceHandle&& other)
+    : handle_(other.handle_), context_(std::move(other.context_)) {
+  other.handle_ = nullptr;
+}
+
+ScopedLibusbDeviceHandle::~ScopedLibusbDeviceHandle() {
+  Reset();
+}
+
+void ScopedLibusbDeviceHandle::Reset() {
+  libusb_close(handle_);
+  handle_ = nullptr;
+  context_.reset();
+}
+
+bool ScopedLibusbDeviceHandle::IsValid() const {
+  return handle_ != nullptr;
+}
+
+}  // namespace device

device/usb/scoped_libusb_device_handle.h

+// Copyright 2018 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.
+
+#ifndef DEVICE_USB_SCOPED_LIBUSB_DEVICE_HANDLE_H_
+#define DEVICE_USB_SCOPED_LIBUSB_DEVICE_HANDLE_H_
+
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+
+struct libusb_device_handle;
+
+namespace device {
+
+class UsbContext;
+
+// This class owns a reference to a libusb_device_handle as well as a reference
+// to the libusb_context. The libusb_context must outlive any
+// libusb_device_handle instances created from it.
+class ScopedLibusbDeviceHandle {
+ public:
+  ScopedLibusbDeviceHandle(libusb_device_handle* handle,
+                           scoped_refptr<UsbContext> context);
+  ScopedLibusbDeviceHandle(ScopedLibusbDeviceHandle&& other);
+  ~ScopedLibusbDeviceHandle();
+
+  libusb_device_handle* get() const { return handle_; }
+
+  void Reset();
+  bool IsValid() const;
+
+ private:
+  libusb_device_handle* handle_;
+  scoped_refptr<UsbContext> context_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedLibusbDeviceHandle);
+};
+
+}  // namespace device
+
+#endif  // DEVICE_USB_SCOPED_LIBUSB_DEVICE_HANDLE_H_

device/usb/scoped_libusb_device_ref.cc

+// Copyright 2018 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/scoped_libusb_device_ref.h"
+
+#include "device/usb/usb_context.h"
+#include "third_party/libusb/src/libusb/libusb.h"
+
+namespace device {
+
+ScopedLibusbDeviceRef::ScopedLibusbDeviceRef(libusb_device* device,
+                                             scoped_refptr<UsbContext> context)
+    : device_(device), context_(std::move(context)) {}
+
+ScopedLibusbDeviceRef::ScopedLibusbDeviceRef(ScopedLibusbDeviceRef&& other)
+    : device_(other.device_), context_(std::move(other.context_)) {
+  other.device_ = nullptr;
+}
+
+ScopedLibusbDeviceRef::~ScopedLibusbDeviceRef() {
+  Reset();
+}
+
+void ScopedLibusbDeviceRef::Reset() {
+  libusb_unref_device(device_);
+  device_ = nullptr;
+  context_.reset();
+}
+
+bool ScopedLibusbDeviceRef::IsValid() const {
+  return device_ != nullptr;
+}
+
+bool operator==(const ScopedLibusbDeviceRef& ref, libusb_device* device) {
+  return ref.get() == device;
+}
+
+}  // namespace device

device/usb/scoped_libusb_device_ref.h

@@ -5,19 +5,40 @@
 #ifndef DEVICE_USB_SCOPED_LIBUSB_DEVICE_REF_H_
 #define DEVICE_USB_SCOPED_LIBUSB_DEVICE_REF_H_
 
-#include "base/scoped_generic.h"
-#include "third_party/libusb/src/libusb/libusb.h"
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+
+struct libusb_device;
 
 namespace device {
 
-struct LibusbDeviceRefTraits {
-  static libusb_device* InvalidValue() { return nullptr; }
+class UsbContext;
+
+// This class owns a reference to a libusb_device as well as a reference to
+// the libusb_context. The libusb_context must outlive any libusb_device
+// instances created from it.
+class ScopedLibusbDeviceRef {
+ public:
+  ScopedLibusbDeviceRef(libusb_device* device,
+                        scoped_refptr<UsbContext> context);
+  ScopedLibusbDeviceRef(ScopedLibusbDeviceRef&& other);
+  ~ScopedLibusbDeviceRef();
+
+  libusb_device* get() const { return device_; }
+
+  scoped_refptr<UsbContext> GetContext() const { return context_; }
+
+  void Reset();
+  bool IsValid() const;
+
+ private:
+  libusb_device* device_;
+  scoped_refptr<UsbContext> context_;
 
-  static void Free(libusb_device* device) { libusb_unref_device(device); }
+  DISALLOW_COPY_AND_ASSIGN(ScopedLibusbDeviceRef);
 };
 
-using ScopedLibusbDeviceRef =
-    base::ScopedGeneric<libusb_device*, LibusbDeviceRefTraits>;
+bool operator==(const ScopedLibusbDeviceRef& ref, libusb_device* device);
 
 }  // namespace device
 

device/usb/usb_device_handle_impl.cc

@@ -311,7 +311,7 @@ UsbDeviceHandleImpl::Transfer::CreateControlTransfer(
   libusb_fill_control_setup(buffer->front(), type, request, value, index,
                             length);
   libusb_fill_control_transfer(transfer->platform_transfer_,
-                               device_handle->handle_, buffer->front(),
+                               device_handle->handle(), buffer->front(),
                                &UsbDeviceHandleImpl::Transfer::PlatformCallback,
                                transfer.get(), timeout);
 
@@ -341,7 +341,7 @@ UsbDeviceHandleImpl::Transfer::CreateBulkTransfer(
   }
 
   libusb_fill_bulk_transfer(
-      transfer->platform_transfer_, device_handle->handle_, endpoint,
+      transfer->platform_transfer_, device_handle->handle(), endpoint,
       buffer->front(), length, &UsbDeviceHandleImpl::Transfer::PlatformCallback,
       transfer.get(), timeout);
 
@@ -371,7 +371,7 @@ UsbDeviceHandleImpl::Transfer::CreateInterruptTransfer(
   }
 
   libusb_fill_interrupt_transfer(
-      transfer->platform_transfer_, device_handle->handle_, endpoint,
+      transfer->platform_transfer_, device_handle->handle(), endpoint,
       buffer->front(), length, &UsbDeviceHandleImpl::Transfer::PlatformCallback,
       transfer.get(), timeout);
 
@@ -401,10 +401,10 @@ UsbDeviceHandleImpl::Transfer::CreateIsochronousTransfer(
     return nullptr;
   }
 
-  libusb_fill_iso_transfer(transfer->platform_transfer_, device_handle->handle_,
-                           endpoint, buffer->front(), static_cast<int>(length),
-                           num_packets, &Transfer::PlatformCallback,
-                           transfer.get(), timeout);
+  libusb_fill_iso_transfer(
+      transfer->platform_transfer_, device_handle->handle(), endpoint,
+      buffer->front(), static_cast<int>(length), num_packets,
+      &Transfer::PlatformCallback, transfer.get(), timeout);
 
   for (size_t i = 0; i < packet_lengths.size(); ++i)
     transfer->platform_transfer_->iso_packet_desc[i].length = packet_lengths[i];
@@ -798,16 +798,14 @@ const UsbInterfaceDescriptor* UsbDeviceHandleImpl::FindInterfaceByEndpoint(
 }
 
 UsbDeviceHandleImpl::UsbDeviceHandleImpl(
-    scoped_refptr<UsbContext> context,
     scoped_refptr<UsbDeviceImpl> device,
-    PlatformUsbDeviceHandle handle,
+    ScopedLibusbDeviceHandle handle,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner)
-    : device_(device),
-      handle_(handle),
-      context_(context),
+    : device_(std::move(device)),
+      handle_(std::move(handle)),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       blocking_task_runner_(blocking_task_runner) {
-  DCHECK(handle) << "Cannot create device with NULL handle.";
+  DCHECK(handle_.IsValid()) << "Cannot create device with an invalid handle.";
 }
 
 UsbDeviceHandleImpl::~UsbDeviceHandleImpl() {
@@ -817,17 +815,19 @@ UsbDeviceHandleImpl::~UsbDeviceHandleImpl() {
   // any thread. libusb is not safe to reentrancy so be sure not to try to close
   // the device from inside a transfer completion callback.
   if (blocking_task_runner_->RunsTasksInCurrentSequence()) {
-    libusb_close(handle_);
+    handle_.Reset();
   } else {
-    blocking_task_runner_->PostTask(FROM_HERE,
-                                    base::BindOnce(&libusb_close, handle_));
+    blocking_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(base::DoNothing::Once<ScopedLibusbDeviceHandle>(),
+                       std::move(handle_)));
   }
 }
 
 void UsbDeviceHandleImpl::SetConfigurationOnBlockingThread(
     int configuration_value,
     ResultCallback callback) {
-  int rv = libusb_set_configuration(handle_, configuration_value);
+  int rv = libusb_set_configuration(handle(), configuration_value);
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to set configuration " << configuration_value
                    << ": " << ConvertPlatformUsbErrorToString(rv);
@@ -855,7 +855,7 @@ void UsbDeviceHandleImpl::SetConfigurationComplete(bool success,
 void UsbDeviceHandleImpl::ClaimInterfaceOnBlockingThread(
     int interface_number,
     ResultCallback callback) {
-  int rv = libusb_claim_interface(handle_, interface_number);
+  int rv = libusb_claim_interface(handle(), interface_number);
   scoped_refptr<InterfaceClaimer> interface_claimer;
   if (rv == LIBUSB_SUCCESS) {
     interface_claimer =
@@ -897,7 +897,7 @@ void UsbDeviceHandleImpl::SetInterfaceAlternateSettingOnBlockingThread(
     int interface_number,
     int alternate_setting,
     ResultCallback callback) {
-  int rv = libusb_set_interface_alt_setting(handle_, interface_number,
+  int rv = libusb_set_interface_alt_setting(handle(), interface_number,
                                             alternate_setting);
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to set interface " << interface_number
@@ -930,7 +930,7 @@ void UsbDeviceHandleImpl::SetInterfaceAlternateSettingComplete(
 }
 
 void UsbDeviceHandleImpl::ResetDeviceOnBlockingThread(ResultCallback callback) {
-  int rv = libusb_reset_device(handle_);
+  int rv = libusb_reset_device(handle());
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to reset device: "
                    << ConvertPlatformUsbErrorToString(rv);
@@ -941,7 +941,7 @@ void UsbDeviceHandleImpl::ResetDeviceOnBlockingThread(ResultCallback callback) {
 
 void UsbDeviceHandleImpl::ClearHaltOnBlockingThread(uint8_t endpoint,
                                                     ResultCallback callback) {
-  int rv = libusb_clear_halt(handle_, endpoint);
+  int rv = libusb_clear_halt(handle(), endpoint);
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to clear halt: "
                    << ConvertPlatformUsbErrorToString(rv);

device/usb/usb_device_handle_impl.h

@@ -17,6 +17,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
+#include "device/usb/scoped_libusb_device_handle.h"
 #include "device/usb/usb_device_handle.h"
 #include "third_party/libusb/src/libusb/libusb.h"
 
@@ -34,10 +35,8 @@ struct EndpointMapValue {
   const UsbEndpointDescriptor* endpoint;
 };
 
-class UsbContext;
 class UsbDeviceImpl;
 
-typedef libusb_device_handle* PlatformUsbDeviceHandle;
 typedef libusb_iso_packet_descriptor* PlatformUsbIsoPacketDescriptor;
 typedef libusb_transfer* PlatformUsbTransferHandle;
 
@@ -90,14 +89,13 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
 
   // This constructor is called by UsbDeviceImpl.
   UsbDeviceHandleImpl(
-      scoped_refptr<UsbContext> context,
       scoped_refptr<UsbDeviceImpl> device,
-      PlatformUsbDeviceHandle handle,
+      ScopedLibusbDeviceHandle handle,
       scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
 
   ~UsbDeviceHandleImpl() override;
 
-  PlatformUsbDeviceHandle handle() const { return handle_; }
+  libusb_device_handle* handle() const { return handle_.get(); }
 
  private:
   class InterfaceClaimer;
@@ -174,7 +172,7 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
 
   scoped_refptr<UsbDeviceImpl> device_;
 
-  PlatformUsbDeviceHandle handle_;
+  ScopedLibusbDeviceHandle handle_;
 
   typedef std::map<int, scoped_refptr<InterfaceClaimer>> ClaimedInterfaceMap;
   ClaimedInterfaceMap claimed_interfaces_;
@@ -186,10 +184,6 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
   typedef std::map<int, EndpointMapValue> EndpointMap;
   EndpointMap endpoint_map_;
 
-  // Retain the UsbContext so that the platform context will not be destroyed
-  // before this handle.
-  scoped_refptr<UsbContext> context_;
-
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
 

device/usb/usb_device_impl.cc

@@ -28,8 +28,7 @@
 
 namespace device {
 
-UsbDeviceImpl::UsbDeviceImpl(scoped_refptr<UsbContext> context,
-                             ScopedLibusbDeviceRef platform_device,
+UsbDeviceImpl::UsbDeviceImpl(ScopedLibusbDeviceRef platform_device,
                              const libusb_device_descriptor& descriptor)
     : UsbDevice(descriptor.bcdUSB,
                 descriptor.bDeviceClass,
@@ -41,9 +40,8 @@ UsbDeviceImpl::UsbDeviceImpl(scoped_refptr<UsbContext> context,
                 base::string16(),
                 base::string16(),
                 base::string16()),
-      context_(std::move(context)),
       platform_device_(std::move(platform_device)) {
-  CHECK(platform_device_.is_valid()) << "platform_device must be valid";
+  CHECK(platform_device_.IsValid()) << "platform_device must be valid";
   ReadAllConfigurations();
   RefreshActiveConfiguration();
 }
@@ -104,12 +102,15 @@ void UsbDeviceImpl::OpenOnBlockingThread(
     scoped_refptr<base::TaskRunner> task_runner,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner) {
   base::AssertBlockingAllowed();
-  PlatformUsbDeviceHandle handle;
+  libusb_device_handle* handle = nullptr;
   const int rv = libusb_open(platform_device(), &handle);
   if (LIBUSB_SUCCESS == rv) {
+    ScopedLibusbDeviceHandle scoped_handle(handle,
+                                           platform_device_.GetContext());
     task_runner->PostTask(
-        FROM_HERE, base::BindOnce(&UsbDeviceImpl::Opened, this, handle,
-                                  std::move(callback), blocking_task_runner));
+        FROM_HERE,
+        base::BindOnce(&UsbDeviceImpl::Opened, this, std::move(scoped_handle),
+                       std::move(callback), blocking_task_runner));
   } else {
     USB_LOG(EVENT) << "Failed to open device: "
                    << ConvertPlatformUsbErrorToString(rv);
@@ -119,12 +120,12 @@ void UsbDeviceImpl::OpenOnBlockingThread(
 }
 
 void UsbDeviceImpl::Opened(
-    PlatformUsbDeviceHandle platform_handle,
+    ScopedLibusbDeviceHandle platform_handle,
     OpenCallback callback,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner) {
   DCHECK(thread_checker_.CalledOnValidThread());
   scoped_refptr<UsbDeviceHandle> device_handle = new UsbDeviceHandleImpl(
-      context_, this, platform_handle, blocking_task_runner);
+      this, std::move(platform_handle), blocking_task_runner);
   handles().push_back(device_handle.get());
   std::move(callback).Run(device_handle);
 }

device/usb/usb_device_impl.h

@@ -21,9 +21,7 @@
 #include "device/usb/usb_descriptors.h"
 #include "device/usb/usb_device.h"
 
-struct libusb_device;
 struct libusb_device_descriptor;
-struct libusb_device_handle;
 
 namespace base {
 class SequencedTaskRunner;
@@ -31,15 +29,12 @@ class SequencedTaskRunner;
 
 namespace device {
 
+class ScopedLibusbDeviceHandle;
 class UsbDeviceHandleImpl;
-class UsbContext;
-
-typedef struct libusb_device_handle* PlatformUsbDeviceHandle;
 
 class UsbDeviceImpl : public UsbDevice {
  public:
-  UsbDeviceImpl(scoped_refptr<UsbContext> context,
-                ScopedLibusbDeviceRef platform_device,
+  UsbDeviceImpl(ScopedLibusbDeviceRef platform_device,
                 const libusb_device_descriptor& descriptor);
 
   // UsbDevice implementation:
@@ -79,15 +74,13 @@ class UsbDeviceImpl : public UsbDevice {
       OpenCallback callback,
       scoped_refptr<base::TaskRunner> task_runner,
       scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
-  void Opened(PlatformUsbDeviceHandle platform_handle,
+  void Opened(ScopedLibusbDeviceHandle platform_handle,
               OpenCallback callback,
               scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
 
   base::ThreadChecker thread_checker_;
   bool visited_ = false;
 
-  // The libusb_context must not be released before the libusb_device.
-  const scoped_refptr<UsbContext> context_;
   const ScopedLibusbDeviceRef platform_device_;
 
   DISALLOW_COPY_AND_ASSIGN(UsbDeviceImpl);

device/usb/usb_service_impl.cc

@@ -131,7 +131,7 @@ void GetDeviceListOnBlockingThread(
   std::vector<ScopedLibusbDeviceRef> scoped_devices;
   scoped_devices.reserve(device_count);
   for (ssize_t i = 0; i < device_count; ++i)
-    scoped_devices.emplace_back(platform_devices[i]);
+    scoped_devices.emplace_back(platform_devices[i], usb_context);
 
   // Free the list but don't unref the devices because ownership has been
   // been transfered to the elements of |scoped_devices|.
@@ -231,6 +231,7 @@ UsbServiceImpl::UsbServiceImpl()
       device_observer_(this),
 #endif
       weak_factory_(this) {
+  weak_self_ = weak_factory_.GetWeakPtr();
   base::PostTaskWithTraits(
       FROM_HERE, kBlockingTaskTraits,
       base::Bind(&InitializeUsbContextOnBlockingThread, task_runner(),
@@ -361,7 +362,7 @@ void UsbServiceImpl::OnDeviceList(
       // Mark the existing device object visited and remove it from the list so
       // it will not be ignored.
       it->second->set_visited(true);
-      device.reset();
+      device.Reset();
     }
   }
 
@@ -382,7 +383,7 @@ void UsbServiceImpl::OnDeviceList(
   // that have been removed don't remain in |ignored_devices_| indefinitely.
   ignored_devices_.clear();
   for (auto& device : *devices) {
-    if (device.is_valid())
+    if (device.IsValid())
       ignored_devices_.push_back(std::move(device));
   }
 
@@ -441,8 +442,8 @@ void UsbServiceImpl::EnumerateDevice(ScopedLibusbDeviceRef platform_device,
 
   devices_being_enumerated_.insert(platform_device.get());
 
-  auto device = base::MakeRefCounted<UsbDeviceImpl>(
-      context_, std::move(platform_device), descriptor);
+  auto device = base::MakeRefCounted<UsbDeviceImpl>(std::move(platform_device),
+                                                    descriptor);
   base::OnceClosure add_device =
       base::BindOnce(&UsbServiceImpl::AddDevice, weak_factory_.GetWeakPtr(),
                      refresh_complete, device);
@@ -458,7 +459,8 @@ void UsbServiceImpl::EnumerateDevice(ScopedLibusbDeviceRef platform_device,
     libusb_ref_device(device->platform_device());
     base::OnceClosure enumeration_failed = base::BindOnce(
         &UsbServiceImpl::EnumerationFailed, weak_factory_.GetWeakPtr(),
-        ScopedLibusbDeviceRef(device->platform_device()), refresh_complete);
+        ScopedLibusbDeviceRef(device->platform_device(), context_),
+        refresh_complete);
 
     device->Open(base::BindOnce(
         &OnDeviceOpenedReadDescriptors, descriptor.iManufacturer,
@@ -518,18 +520,18 @@ int LIBUSB_CALL UsbServiceImpl::HotplugCallback(libusb_context* context,
   // libusb does not transfer ownership of |device_raw| to this function so a
   // reference must be taken here.
   libusb_ref_device(device_raw);
-  ScopedLibusbDeviceRef device(device_raw);
+  ScopedLibusbDeviceRef device(device_raw, self->context_);
 
   switch (event) {
     case LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED:
       self->task_runner()->PostTask(
           FROM_HERE, base::BindOnce(&UsbServiceImpl::OnPlatformDeviceAdded,
-                                    base::Unretained(self), std::move(device)));
+                                    self->weak_self_, std::move(device)));
       break;
     case LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT:
       self->task_runner()->PostTask(
           FROM_HERE, base::BindOnce(&UsbServiceImpl::OnPlatformDeviceRemoved,
-                                    base::Unretained(self), std::move(device)));
+                                    self->weak_self_, std::move(device)));
       break;
     default:
       NOTREACHED();

device/usb/usb_service_impl.h

@@ -122,6 +122,10 @@ class UsbServiceImpl :
   ScopedObserver<DeviceMonitorWin, DeviceMonitorWin::Observer> device_observer_;
 #endif  // OS_WIN
 
+  // This WeakPtr is used to safely post hotplug events back to the thread this
+  // object lives on.
+  base::WeakPtr<UsbServiceImpl> weak_self_;
+
   base::WeakPtrFactory<UsbServiceImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(UsbServiceImpl);