From 13856b239a3585f4c9b848b2c79824efe7829d24 Mon Sep 17 00:00:00 2001
From: Myles Watson <mylesgw@google.com>
Date: Tue, 11 Jun 2019 13:46:15 -0700
Subject: [PATCH] HAL: Add unregisterIncomingPacketCallback

Test: bluetooth_test_gd
Change-Id: I17f8227c351451b4249c2a005ea4428ace734dc9
---
 gd/hal/hci_hal.h                      |  8 ++++----
 gd/hal/hci_hal_android_hidl.cc        |  4 ++++
 gd/hal/hci_hal_host_rootcanal.cc      | 10 +++++++++-
 gd/hal/hci_hal_host_rootcanal_test.cc |  1 +
 gd/hci/hci_layer.cc                   |  1 +
 gd/hci/hci_layer_test.cc              | 16 ++++++++++++----
 6 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/gd/hal/hci_hal.h b/gd/hal/hci_hal.h
index 88c0d9ecb..f95a4f9d1 100644
--- a/gd/hal/hci_hal.h
+++ b/gd/hal/hci_hal.h
@@ -63,16 +63,16 @@ class HciHal : public ::bluetooth::Module {
   virtual ~HciHal() = default;
 
   // Register the callback for incoming packets. All incoming packets are dropped before
-  // this callback is registered. Callback can only be registered once, but will be reset
-  // after close().
-  //
-  // Call this function before initialize() to guarantee all incoming packets are received.
+  // this callback is registered. Callback can only be registered once.
   //
   // @param callback implements BluetoothHciHalCallbacks which will
   //    receive callbacks when incoming HCI packets are received
   //    from the controller to be sent to the host.
   virtual void registerIncomingPacketCallback(HciHalCallbacks* callback) = 0;
 
+  // Unregister the callback for incoming packets. Drop all further incoming packets.
+  virtual void unregisterIncomingPacketCallback() = 0;
+
   // Send an HCI command (as specified in the Bluetooth Specification
   // V4.2, Vol 2, Part 5, Section 5.4.1) to the Bluetooth controller.
   // Commands must be executed in order.
diff --git a/gd/hal/hci_hal_android_hidl.cc b/gd/hal/hci_hal_android_hidl.cc
index 893583747..195486241 100644
--- a/gd/hal/hci_hal_android_hidl.cc
+++ b/gd/hal/hci_hal_android_hidl.cc
@@ -120,6 +120,10 @@ class HciHalHidl : public HciHal {
     callbacks_->SetCallback(callback);
   }
 
+  void unregisterIncomingPacketCallback() override {
+    callbacks_->ResetCallback();
+  }
+
   void sendHciCommand(HciPacket command) override {
     btsnoop_logger_->capture(command, SnoopLogger::Direction::OUTGOING, SnoopLogger::PacketType::CMD);
     bt_hci_->sendHciCommand(command);
diff --git a/gd/hal/hci_hal_host_rootcanal.cc b/gd/hal/hci_hal_host_rootcanal.cc
index b6a2948df..f8e4c61fb 100644
--- a/gd/hal/hci_hal_host_rootcanal.cc
+++ b/gd/hal/hci_hal_host_rootcanal.cc
@@ -96,6 +96,11 @@ class HciHalHostRootcanal : public HciHal {
     incoming_packet_callback_ = callback;
   }
 
+  void unregisterIncomingPacketCallback() override {
+    std::lock_guard<std::mutex> lock(mutex_);
+    incoming_packet_callback_ = nullptr;
+  }
+
   void sendHciCommand(HciPacket command) override {
     std::lock_guard<std::mutex> lock(mutex_);
     ASSERT(sock_fd_ != INVALID_FD);
@@ -190,7 +195,10 @@ class HciHalHostRootcanal : public HciHal {
   }
 
   void incoming_packet_received() {
-    ASSERT(incoming_packet_callback_ != nullptr);
+    if (incoming_packet_callback_ == nullptr) {
+      LOG_INFO("Dropping a packet");
+      return;
+    }
 
     uint8_t buf[kBufSize] = {};
 
diff --git a/gd/hal/hci_hal_host_rootcanal_test.cc b/gd/hal/hci_hal_host_rootcanal_test.cc
index ae9873794..e06c80335 100644
--- a/gd/hal/hci_hal_host_rootcanal_test.cc
+++ b/gd/hal/hci_hal_host_rootcanal_test.cc
@@ -147,6 +147,7 @@ class HciHalRootcanalTest : public ::testing::Test {
   }
 
   void TearDown() override {
+    hal_->unregisterIncomingPacketCallback();
     fake_registry_.StopAll();
     close(fake_server_socket_);
     delete fake_server_;
diff --git a/gd/hci/hci_layer.cc b/gd/hci/hci_layer.cc
index 02211ff12..93c719117 100644
--- a/gd/hci/hci_layer.cc
+++ b/gd/hci/hci_layer.cc
@@ -113,6 +113,7 @@ struct HciLayer::impl : public hal::HciHalCallbacks {
   }
 
   void Stop() {
+    hal_->unregisterIncomingPacketCallback();
     acl_queue_.GetDownEnd()->UnregisterDequeue();
     delete hci_timeout_alarm_;
     command_queue_.clear();
diff --git a/gd/hci/hci_layer_test.cc b/gd/hci/hci_layer_test.cc
index 4cca090be..f58cb0818 100644
--- a/gd/hci/hci_layer_test.cc
+++ b/gd/hci/hci_layer_test.cc
@@ -52,19 +52,27 @@ class TestHciHal : public hal::HciHal {
  public:
   TestHciHal() : hal::HciHal() {}
 
-  virtual void registerIncomingPacketCallback(hal::HciHalCallbacks* callback) {
+  ~TestHciHal() {
+    ASSERT_LOG(callbacks == nullptr, "unregisterIncomingPacketCallback() must be called");
+  }
+
+  void registerIncomingPacketCallback(hal::HciHalCallbacks* callback) override {
     callbacks = callback;
   }
 
-  virtual void sendHciCommand(hal::HciPacket command) {
+  void unregisterIncomingPacketCallback() override {
+    callbacks = nullptr;
+  }
+
+  void sendHciCommand(hal::HciPacket command) override {
     outgoing_commands_.push_back(std::move(command));
   }
 
-  virtual void sendAclData(hal::HciPacket data) {
+  void sendAclData(hal::HciPacket data) override {
     outgoing_acl_.push_front(std::move(data));
   }
 
-  virtual void sendScoData(hal::HciPacket data) {
+  void sendScoData(hal::HciPacket data) override {
     outgoing_sco_.push_front(std::move(data));
   }
 
-- 
2.11.0