L2cap: Don't erase link when outgoing packet pending

Otherwise we will post to L2cap handler DataController::OnSdu with
deleted object.

Test: LE mouse
Bug: 179326888
Tag: #gd-refactor
Change-Id: I517ac9890cc8d3894067bcad346a80765d24aeca

diff --git a/gd/l2cap/classic/internal/link.cc b/gd/l2cap/classic/internal/link.cc
index be84dac..5167cc1 100644 (file)
--- a/gd/l2cap/classic/internal/link.cc
+++ b/gd/l2cap/classic/internal/link.cc
@@ -452,6 +452,17 @@ void Link::AddEncryptionChangeListener(EncryptionChangeListener listener) {
   encryption_change_listener_.push_back(listener);
 }
 
+void Link::OnPendingPacketChange(Cid local_cid, bool has_packet) {
+  if (has_packet) {
+    remaining_packets_to_be_sent_++;
+  } else {
+    remaining_packets_to_be_sent_--;
+  }
+  if (link_manager_ != nullptr) {
+    link_manager_->OnPendingPacketChange(GetDevice().GetAddress(), remaining_packets_to_be_sent_);
+  }
+}
+
 }  // namespace internal
 }  // namespace classic
 }  // namespace l2cap

diff --git a/gd/l2cap/classic/internal/link.h b/gd/l2cap/classic/internal/link.h
index 4696741..6121b4f 100644 (file)
--- a/gd/l2cap/classic/internal/link.h
+++ b/gd/l2cap/classic/internal/link.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <atomic>
 #include <memory>
 #include <unordered_map>
 
@@ -199,6 +200,8 @@ class Link : public l2cap::internal::ILink, public hci::acl_manager::ConnectionM
     return role_;
   }
 
+  void OnPendingPacketChange(Cid local_cid, bool has_packet) override;
+
  private:
   friend class DumpsysHelper;
   void connect_to_pending_dynamic_channels();
@@ -229,6 +232,7 @@ class Link : public l2cap::internal::ILink, public hci::acl_manager::ConnectionM
   bool used_by_security_module_ = false;
   bool has_requested_authentication_ = false;
   std::list<EncryptionChangeListener> encryption_change_listener_;
+  std::atomic_int remaining_packets_to_be_sent_ = 0;
   DISALLOW_COPY_AND_ASSIGN(Link);
 };
 

diff --git a/gd/l2cap/classic/internal/link_manager.cc b/gd/l2cap/classic/internal/link_manager.cc
index 22dee47..a80216e 100644 (file)
--- a/gd/l2cap/classic/internal/link_manager.cc
+++ b/gd/l2cap/classic/internal/link_manager.cc
@@ -123,6 +123,17 @@ void LinkManager::RegisterLinkPropertyListener(os::Handler* handler, LinkPropert
   link_property_listener_ = listener;
 }
 
+void LinkManager::OnPendingPacketChange(hci::Address remote, int num_packets) {
+  if (disconnected_links_.count(remote) != 0 && num_packets == 0) {
+    links_.erase(remote);
+    links_with_pending_packets_.erase(remote);
+  } else if (num_packets != 0) {
+    links_with_pending_packets_.emplace(remote);
+  } else {
+    links_with_pending_packets_.erase(remote);
+  }
+}
+
 Link* LinkManager::GetLink(const hci::Address device) {
   if (links_.find(device) == links_.end()) {
     return nullptr;
@@ -314,7 +325,11 @@ void LinkManager::OnDisconnect(hci::Address device, hci::ErrorCode status) {
     link_property_callback_handler_->CallOn(link_property_listener_, &LinkPropertyListener::OnLinkDisconnected, device);
   }
 
-  links_.erase(device);
+  if (links_with_pending_packets_.count(device) != 0) {
+    disconnected_links_.emplace(device);
+  } else {
+    links_.erase(device);
+  }
 }
 
 void LinkManager::OnAuthenticationComplete(hci::ErrorCode hci_status, hci::Address device) {

diff --git a/gd/l2cap/classic/internal/link_manager.h b/gd/l2cap/classic/internal/link_manager.h
index 95bb0de..1726226 100644 (file)
--- a/gd/l2cap/classic/internal/link_manager.h
+++ b/gd/l2cap/classic/internal/link_manager.h
@@ -108,6 +108,10 @@ class LinkManager : public hci::acl_manager::ConnectionCallbacks {
   // Registerlink callbacks
   void RegisterLinkPropertyListener(os::Handler* handler, LinkPropertyListener* listener);
 
+  // Reported by link to indicate how many pending packets are remaining to be set.
+  // If there is anything outstanding, don't delete link
+  void OnPendingPacketChange(hci::Address remote, int num_packets);
+
  private:
   // Handles requests from LinkSecurityInterface
   friend class LinkSecurityInterfaceImpl;
@@ -135,6 +139,8 @@ class LinkManager : public hci::acl_manager::ConnectionCallbacks {
   LinkSecurityInterfaceListener* link_security_interface_listener_ = nullptr;
   LinkPropertyListener* link_property_listener_ = nullptr;
   os::Handler* link_property_callback_handler_ = nullptr;
+  std::unordered_set<hci::Address> disconnected_links_;
+  std::unordered_set<hci::Address> links_with_pending_packets_;
 
   DISALLOW_COPY_AND_ASSIGN(LinkManager);
 };

diff --git a/gd/l2cap/internal/ilink.h b/gd/l2cap/internal/ilink.h
index 4f6d664..dfac779 100644 (file)
--- a/gd/l2cap/internal/ilink.h
+++ b/gd/l2cap/internal/ilink.h
@@ -32,6 +32,10 @@ class ILink {
   virtual void SendDisconnectionRequest(Cid local_cid, Cid remote_cid) = 0;
   virtual hci::AddressWithType GetDevice() const = 0;
 
+  // Used by sender to indicate whether there is any pending packet to be sent.
+  // If there is pending packet, don't delete the link.
+  virtual void OnPendingPacketChange(Cid local_cid, bool has_packet) {}
+
   // To be used by LE credit based channel data controller over LE link
   virtual void SendLeCredit(Cid local_cid, uint16_t credit) {}
 

diff --git a/gd/l2cap/internal/sender.cc b/gd/l2cap/internal/sender.cc
index 2c740ca..d9ef716 100644 (file)
--- a/gd/l2cap/internal/sender.cc
+++ b/gd/l2cap/internal/sender.cc
@@ -61,6 +61,7 @@ Sender::~Sender() {
 }
 
 void Sender::OnPacketSent() {
+  link_->OnPendingPacketChange(channel_id_, false);
   try_register_dequeue();
 }
 
@@ -88,6 +89,7 @@ void Sender::dequeue_callback() {
   if (is_dequeue_registered_.exchange(false)) {
     queue_end_->UnregisterDequeue();
   }
+  link_->OnPendingPacketChange(channel_id_, true);
 }
 
 void Sender::UpdateClassicConfiguration(classic::internal::ChannelConfigurationState config) {

diff --git a/gd/l2cap/internal/sender_test.cc b/gd/l2cap/internal/sender_test.cc
index bd24fd4..99a9b31 100644 (file)
--- a/gd/l2cap/internal/sender_test.cc
+++ b/gd/l2cap/internal/sender_test.cc
@@ -21,6 +21,7 @@
 #include <future>
 
 #include "l2cap/internal/channel_impl_mock.h"
+#include "l2cap/internal/ilink_mock.h"
 #include "l2cap/internal/scheduler.h"
 #include "os/handler.h"
 #include "os/queue.h"
@@ -77,7 +78,7 @@ class L2capSenderTest : public ::testing::Test {
     EXPECT_CALL(*mock_channel_, GetQueueDownEnd()).WillRepeatedly(Return(channel_queue_.GetDownEnd()));
     EXPECT_CALL(*mock_channel_, GetCid()).WillRepeatedly(Return(cid_));
     EXPECT_CALL(*mock_channel_, GetRemoteCid()).WillRepeatedly(Return(cid_));
-    sender_ = new Sender(queue_handler_, nullptr, &scheduler_, mock_channel_);
+    sender_ = new Sender(queue_handler_, &link_, &scheduler_, mock_channel_);
   }
 
   void TearDown() override {
@@ -97,6 +98,7 @@ class L2capSenderTest : public ::testing::Test {
   Sender* sender_ = nullptr;
   Cid cid_ = 0x41;
   FakeScheduler scheduler_;
+  testing::MockILink link_;
 };
 
 TEST_F(L2capSenderTest, send_packet) {

diff --git a/gd/l2cap/le/internal/link.cc b/gd/l2cap/le/internal/link.cc
index 0cb0c7d..be759b2 100644 (file)
--- a/gd/l2cap/le/internal/link.cc
+++ b/gd/l2cap/le/internal/link.cc
@@ -294,6 +294,15 @@ void Link::on_connection_update_complete(SignalId signal_id, hci::ErrorCode erro
   signalling_manager_.SendConnectionParameterUpdateResponse(SignalId(), result);
 }
 
+void Link::OnPendingPacketChange(Cid local_cid, bool has_packet) {
+  if (has_packet) {
+    remaining_packets_to_be_sent_++;
+  } else {
+    remaining_packets_to_be_sent_--;
+  }
+  link_manager_->OnPendingPacketChange(GetDevice(), remaining_packets_to_be_sent_);
+}
+
 }  // namespace internal
 }  // namespace le
 }  // namespace l2cap

diff --git a/gd/l2cap/le/internal/link.h b/gd/l2cap/le/internal/link.h
index 46140e6..518793e 100644 (file)
--- a/gd/l2cap/le/internal/link.h
+++ b/gd/l2cap/le/internal/link.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <atomic>
 #include <chrono>
 #include <memory>
 
@@ -148,6 +149,8 @@ class Link : public l2cap::internal::ILink, public hci::acl_manager::LeConnectio
 
   void ReadRemoteVersionInformation();
 
+  void OnPendingPacketChange(Cid local_cid, bool has_packet) override;
+
  private:
   os::Handler* l2cap_handler_;
   l2cap::internal::FixedChannelAllocator<FixedChannelImpl, Link> fixed_channel_allocator_{this, l2cap_handler_};
@@ -166,6 +169,7 @@ class Link : public l2cap::internal::ILink, public hci::acl_manager::LeConnectio
   uint16_t update_request_interval_max_;
   uint16_t update_request_latency_;
   uint16_t update_request_supervision_timeout_;
+  std::atomic_int remaining_packets_to_be_sent_ = 0;
   DISALLOW_COPY_AND_ASSIGN(Link);
 
   // Received connection update complete from ACL manager. SignalId is bound to a valid number when we need to send a

diff --git a/gd/l2cap/le/internal/link_manager.cc b/gd/l2cap/le/internal/link_manager.cc
index 1a4d8df..b41832d 100644 (file)
--- a/gd/l2cap/le/internal/link_manager.cc
+++ b/gd/l2cap/le/internal/link_manager.cc
@@ -149,7 +149,11 @@ void LinkManager::OnDisconnect(bluetooth::hci::AddressWithType address_with_type
   auto* link = GetLink(address_with_type);
   ASSERT_LOG(link != nullptr, "Device %s is disconnected but not in local database",
              address_with_type.ToString().c_str());
-  links_.erase(address_with_type);
+  if (links_with_pending_packets_.count(address_with_type) != 0) {
+    disconnected_links_.emplace(address_with_type);
+  } else {
+    links_.erase(address_with_type);
+  }
 
   if (link_property_callback_handler_ != nullptr) {
     link_property_callback_handler_->CallOn(
@@ -196,6 +200,17 @@ void LinkManager::OnReadRemoteVersionInformationComplete(
   }
 }
 
+void LinkManager::OnPendingPacketChange(hci::AddressWithType remote, int num_packets) {
+  if (disconnected_links_.count(remote) != 0 && num_packets == 0) {
+    links_.erase(remote);
+    links_with_pending_packets_.erase(remote);
+  } else if (num_packets != 0) {
+    links_with_pending_packets_.emplace(remote);
+  } else {
+    links_with_pending_packets_.erase(remote);
+  }
+}
+
 }  // namespace internal
 }  // namespace le
 }  // namespace l2cap

diff --git a/gd/l2cap/le/internal/link_manager.h b/gd/l2cap/le/internal/link_manager.h
index da70354..c6ec046 100644 (file)
--- a/gd/l2cap/le/internal/link_manager.h
+++ b/gd/l2cap/le/internal/link_manager.h
@@ -18,6 +18,7 @@
 
 #include <memory>
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
 
 #include "os/handler.h"
@@ -94,6 +95,10 @@ class LinkManager : public hci::acl_manager::LeConnectionCallbacks {
       uint16_t manufacturer_name,
       uint16_t sub_version);
 
+  // Reported by link to indicate how many pending packets are remaining to be set.
+  // If there is anything outstanding, don't delete link
+  void OnPendingPacketChange(hci::AddressWithType remote, int num_packets);
+
  private:
   // Dependencies
   os::Handler* l2cap_handler_;
@@ -109,6 +114,8 @@ class LinkManager : public hci::acl_manager::LeConnectionCallbacks {
       pending_dynamic_channels_;
   os::Handler* link_property_callback_handler_ = nullptr;
   LinkPropertyListener* link_property_listener_ = nullptr;
+  std::unordered_set<hci::AddressWithType> disconnected_links_;
+  std::unordered_set<hci::AddressWithType> links_with_pending_packets_;
 
   DISALLOW_COPY_AND_ASSIGN(LinkManager);
 };