Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit caec024c authored by Rahul Arya's avatar Rahul Arya
Browse files

Add Remote Name Request scheduling to ACL scheduler 

Remote name requests need to be queued along with ACL Create Connection
requests to avoid the commands being rejected by the controller. This
adds the relevant hooks into ACL scheduler to do so, analogous to those
that exist for connection creation.

Test: gd unit tests
Bug: 246640776
Tag: #refactor
BYPASS_LONG_LINES_REASON: Bluetooth

Change-Id: Ia6ce5d598eddb771110df44375f9f8256e411132
parent 798ed5e0

system/gd/hci/acl_manager/acl_scheduler.cc

+137 −33
Original line number Diff line number Diff line
@@ -16,8 +16,10 @@ 


#include "acl_scheduler.h"



#include <optional>

#include <queue>

#include <unordered_set>

#include <variant>



namespace bluetooth {

namespace hci {
@@ -29,10 +31,18 @@ struct AclCreateConnectionQueueEntry { 
  common::ContextualOnceCallback<void()> callback;

};



struct RemoteNameRequestQueueEntry {

  Address address;

  common::ContextualOnceCallback<void()> callback;

  common::ContextualOnceCallback<void()> callback_when_cancelled;

};



using QueueEntry = std::variant<AclCreateConnectionQueueEntry, RemoteNameRequestQueueEntry>;



struct AclScheduler::impl {

  void EnqueueOutgoingAclConnection(Address address, common::ContextualOnceCallback<void()> start_connection) {

    pending_outgoing_connections_.push_back({address, std::move(start_connection)});

    try_dequeue_next_connection();

    pending_outgoing_operations_.push_back(AclCreateConnectionQueueEntry{address, std::move(start_connection)});

    try_dequeue_next_operation();

  }



  void RegisterPendingIncomingConnection(Address address) {
@@ -44,68 +54,141 @@ struct AclScheduler::impl { 
      common::ContextualOnceCallback<void()> handle_outgoing_connection,

      common::ContextualOnceCallback<void()> handle_incoming_connection,

      common::ContextualOnceCallback<void(std::string)> handle_unknown_connection) {

    if (outgoing_connecting_address_ == address) {

      outgoing_connecting_address_ = Address::kEmpty;

    // Check if an outgoing request (a) exists, (b) is a Create Connection, (c) matches the received address

    if (outgoing_entry_.has_value()) {

      auto entry = std::get_if<AclCreateConnectionQueueEntry>(&outgoing_entry_.value());

      if (entry != nullptr && entry->address == address) {

        // If so, clear the current entry and advance the queue

        outgoing_entry_.reset();

        handle_outgoing_connection.InvokeIfNotEmpty();

    } else if (incoming_connecting_address_set_.find(address) != incoming_connecting_address_set_.end()) {

        try_dequeue_next_operation();

        return;

      }

    }



    // Otherwise check if it's an incoming request and advance the queue if so

    if (incoming_connecting_address_set_.find(address) != incoming_connecting_address_set_.end()) {

      incoming_connecting_address_set_.erase(address);

      handle_incoming_connection.InvokeIfNotEmpty();

    } else {

      handle_unknown_connection.InvokeIfNotEmpty(set_of_incoming_connecting_addresses());

    }

    try_dequeue_next_connection();

    try_dequeue_next_operation();

  }



  void ReportOutgoingAclConnectionFailure() {

    if (outgoing_connecting_address_ == Address::kEmpty) {

    if (!outgoing_entry_.has_value()) {

      LOG_ERROR("Outgoing connection failure reported, but none present!");

      return;

    }

    outgoing_connecting_address_ = Address::kEmpty;

    try_dequeue_next_connection();

    auto entry = std::get_if<AclCreateConnectionQueueEntry>(&outgoing_entry_.value());

    if (entry == nullptr) {

      LOG_ERROR("Outgoing connection failure reported, but we're currently doing an RNR!");

      return;

    }

    outgoing_entry_.reset();

    try_dequeue_next_operation();

  }



  void CancelAclConnection(

      Address address,

      common::ContextualOnceCallback<void()> cancel_connection,

      common::ContextualOnceCallback<void()> cancel_connection_completed) {

    // Check if relevant connection is currently outgoing

    if (outgoing_connecting_address_ == address) {

      cancel_connection.Invoke();

      // as per method contract, we *don't* clear it from the queue

      return;

    auto ok = cancel_outgoing_or_queued_connection(

        [&](auto& entry) {

          auto entry_ptr = std::get_if<AclCreateConnectionQueueEntry>(&entry);

          return entry_ptr != nullptr && entry_ptr->address == address;

        },

        [&]() { cancel_connection.Invoke(); },

        [&](auto entry) { cancel_connection_completed.Invoke(); });

    if (!ok) {

      LOG_ERROR("Attempted to cancel connection to %s that does not exist", address.ToString().c_str());

    }

  }



    // Otherwise, clear from the queue

    auto it =

        std::find_if(pending_outgoing_connections_.begin(), pending_outgoing_connections_.end(), [&](auto& entry) {

          return entry.address == address;

        });

    if (it == pending_outgoing_connections_.end()) {

      LOG_ERROR("Attempted to cancel connection to %s that does not exist", address.ToString().c_str());

  void EnqueueRemoteNameRequest(

      Address address,

      common::ContextualOnceCallback<void()> start_request,

      common::ContextualOnceCallback<void()> cancel_request_completed) {

    pending_outgoing_operations_.push_back(

        RemoteNameRequestQueueEntry{address, std::move(start_request), std::move(cancel_request_completed)});

    try_dequeue_next_operation();

  }



  void ReportRemoteNameRequestCompletion(Address address) {

    if (!outgoing_entry_.has_value()) {

      LOG_ERROR("Remote name request completion reported, but none taking place!");

      return;

    }

    pending_outgoing_connections_.erase(it);

    cancel_connection_completed.Invoke();



    std::visit(

        [](auto&& entry) {

          using T = std::decay_t<decltype(entry)>;

          if constexpr (std::is_same_v<T, RemoteNameRequestQueueEntry>) {

            LOG_INFO("Remote name request completed");

          } else if constexpr (std::is_same_v<T, AclCreateConnectionQueueEntry>) {

            LOG_ERROR(

                "Received RNR completion when ACL connection is outstanding - assuming the connection has failed and "

                "continuing");

          } else {

            static_assert(!sizeof(T*), "non-exhaustive visitor!");

          }

        },

        outgoing_entry_.value());



    outgoing_entry_.reset();

    try_dequeue_next_operation();

  }



  void CancelRemoteNameRequest(Address address, common::ContextualOnceCallback<void()> cancel_request) {

    auto ok = cancel_outgoing_or_queued_connection(

        [&](auto& entry) {

          auto entry_ptr = std::get_if<RemoteNameRequestQueueEntry>(&entry);

          return entry_ptr != nullptr && entry_ptr->address == address;

        },

        [&]() { cancel_request.Invoke(); },

        [](auto entry) { std::get<RemoteNameRequestQueueEntry>(entry).callback_when_cancelled.Invoke(); });

    if (!ok) {

      LOG_ERROR("Attempted to cancel remote name request to %s that does not exist", address.ToString().c_str());

    }

  };



  void Stop() {

    stopped_ = true;

  }



 private:

  void try_dequeue_next_connection() {

  void try_dequeue_next_operation() {

    if (stopped_) {

      return;

    }

    if (incoming_connecting_address_set_.empty() && outgoing_connecting_address_.IsEmpty() &&

        !pending_outgoing_connections_.empty()) {

    if (incoming_connecting_address_set_.empty() && !outgoing_entry_.has_value() &&

        !pending_outgoing_operations_.empty()) {

      LOG_INFO("Pending connections is not empty; so sending next connection");

      auto entry = std::move(pending_outgoing_connections_.front());

      pending_outgoing_connections_.pop_front();

      outgoing_connecting_address_ = entry.address;

      entry.callback.Invoke();

      auto entry = std::move(pending_outgoing_operations_.front());

      pending_outgoing_operations_.pop_front();

      std::visit([](auto&& variant) { variant.callback.Invoke(); }, entry);

      outgoing_entry_ = std::move(entry);

    }

  }



  template <typename T, typename U, typename V>

  bool cancel_outgoing_or_queued_connection(T matcher, U cancel_outgoing, V cancelled_queued) {

    // Check if relevant connection is currently outgoing

    if (outgoing_entry_.has_value()) {

      if (matcher(outgoing_entry_.value())) {

        cancel_outgoing();

        return true;

      }

    }

    // Otherwise, clear from the queue

    auto it = std::find_if(pending_outgoing_operations_.begin(), pending_outgoing_operations_.end(), matcher);

    if (it == pending_outgoing_operations_.end()) {

      return false;

    }

    cancelled_queued(std::move(*it));

    pending_outgoing_operations_.erase(it);

    return true;

  }



  const std::string set_of_incoming_connecting_addresses() const {
@@ -114,8 +197,8 @@ struct AclScheduler::impl { 
    return buffer.str();

  }



  Address outgoing_connecting_address_;

  std::deque<AclCreateConnectionQueueEntry> pending_outgoing_connections_;

  std::optional<QueueEntry> outgoing_entry_;

  std::deque<QueueEntry> pending_outgoing_operations_;

  std::unordered_set<Address> incoming_connecting_address_set_;

  bool stopped_ = false;

};
@@ -165,6 +248,27 @@ void AclScheduler::CancelAclConnection( 
      std::move(cancel_connection_completed));

}



void AclScheduler::EnqueueRemoteNameRequest(

    Address address,

    common::ContextualOnceCallback<void()> start_request,

    common::ContextualOnceCallback<void()> cancel_request_completed) {

  GetHandler()->Call(

      &impl::EnqueueRemoteNameRequest,

      common::Unretained(pimpl_.get()),

      address,

      std::move(start_request),

      std::move(cancel_request_completed));

}



void AclScheduler::ReportRemoteNameRequestCompletion(Address address) {

  GetHandler()->Call(&impl::ReportRemoteNameRequestCompletion, common::Unretained(pimpl_.get()), address);

}



void AclScheduler::CancelRemoteNameRequest(Address address, common::ContextualOnceCallback<void()> cancel_request) {

  GetHandler()->Call(

      &impl::CancelRemoteNameRequest, common::Unretained(pimpl_.get()), address, std::move(cancel_request));

}



void AclScheduler::ListDependencies(ModuleList* list) const {}



void AclScheduler::Start() {}

system/gd/hci/acl_manager/acl_scheduler.h

+15 −0
Original line number Diff line number Diff line
@@ -63,6 +63,21 @@ class AclScheduler : public bluetooth::Module { 
      common::ContextualOnceCallback<void()> cancel_connection,

      common::ContextualOnceCallback<void()> cancel_connection_completed);



  // Schedule a Remote Name Request. When the request is started, start_request will be invoked. If the request is

  // cancelled before it is dequeued, cancel_request_completed will be invoked.

  void EnqueueRemoteNameRequest(

      Address address,

      common::ContextualOnceCallback<void()> start_request,

      common::ContextualOnceCallback<void()> cancel_request_completed);



  // Report that a Remote Name Request connection has completed, so we can resume popping from the queue.

  void ReportRemoteNameRequestCompletion(Address address);



  // Cancel an Remote Name Request. If the request is already outgoing, we will invoke cancel_request, without

  // clearing the outgoing request. Otherwise, we will invoke the cancel_request_completed callback registered on

  // the initial enqueue.

  void CancelRemoteNameRequest(Address address, common::ContextualOnceCallback<void()> cancel_request);



 private:

  struct impl;

  std::unique_ptr<impl> pimpl_;

system/gd/hci/acl_manager/acl_scheduler_test.cc

+115 −0
Original line number Diff line number Diff line
@@ -359,6 +359,121 @@ TEST_F(AclSchedulerTest, CancelQueuedConnectionCallback) { 
  EXPECT_THAT(future, IsSet());

}



TEST_F(AclSchedulerTest, RemoteNameRequestImmediatelyExecuted) {

  auto promise = std::promise<void>{};

  auto future = promise.get_future();



  // start an outgoing request

  acl_scheduler_->EnqueueRemoteNameRequest(address1, promiseCallback(std::move(promise)), emptyCallback());



  // we expect the start callback to be invoked immediately

  EXPECT_THAT(future, IsSet());

}



TEST_F(AclSchedulerTest, RemoteNameRequestQueuing) {

  auto promise = std::promise<void>{};

  auto future = promise.get_future();



  // start an outgoing request

  acl_scheduler_->EnqueueRemoteNameRequest(address1, emptyCallback(), impossibleCallback());

  // enqueue a second one

  acl_scheduler_->EnqueueRemoteNameRequest(address2, promiseCallback(std::move(promise)), impossibleCallback());



  // we should still be queued

  EXPECT_THAT(future.wait_for(timeout), std::future_status::timeout);



  // the first request completes

  acl_scheduler_->ReportRemoteNameRequestCompletion(address1);



  // so the second request should now have started

  EXPECT_THAT(future, IsSet());

}



TEST_F(AclSchedulerTest, RemoteNameRequestCancellationCallback) {

  auto promise = std::promise<void>{};

  auto future = promise.get_future();



  // start an outgoing request

  acl_scheduler_->EnqueueRemoteNameRequest(address1, emptyCallback(), impossibleCallback());



  // cancel it

  acl_scheduler_->CancelRemoteNameRequest(address1, promiseCallback(std::move(promise)));



  // the cancel callback should be invoked

  EXPECT_THAT(future, IsSet());

}



TEST_F(AclSchedulerTest, RemoteNameRequestCancellationWhileQueuedCallback) {

  auto promise = std::promise<void>{};

  auto future = promise.get_future();



  // start an outgoing request

  acl_scheduler_->EnqueueRemoteNameRequest(address1, emptyCallback(), impossibleCallback());

  // enqueue a second one

  acl_scheduler_->EnqueueRemoteNameRequest(address2, impossibleCallback(), promiseCallback(std::move(promise)));



  // cancel the second one

  acl_scheduler_->CancelRemoteNameRequest(address2, impossibleCallback());



  // the cancel_request_completed calback should be invoked

  EXPECT_THAT(future, IsSet());



  // the first request completes

  acl_scheduler_->ReportRemoteNameRequestCompletion(address1);



  // we don't dequeue the second one, since it was cancelled

  // implicitly assert that its callback was never invoked

}



TEST_F(AclSchedulerTest, CancelQueuedRemoteNameRequestRemoveFromQueue) {

  auto promise = std::promise<void>{};

  auto future = promise.get_future();



  // start an outgoing connection

  acl_scheduler_->EnqueueOutgoingAclConnection(address1, emptyCallback());

  // start another connection that will queue

  acl_scheduler_->EnqueueRemoteNameRequest(address2, impossibleCallback(), emptyCallback());

  // start a third connection that will queue

  acl_scheduler_->EnqueueRemoteNameRequest(address3, promiseCallback(std::move(promise)), impossibleCallback());



  // cancel the first queued connection

  acl_scheduler_->CancelRemoteNameRequest(address2, impossibleCallback());



  // the second queued connection should remain enqueued, since another connection is in progress

  EXPECT_THAT(future.wait_for(timeout), std::future_status::timeout);



  // complete the outgoing connection

  acl_scheduler_->ReportOutgoingAclConnectionFailure();



  // only now can we dequeue the second queued connection

  EXPECT_THAT(future, IsSet());

}



TEST_F(AclSchedulerTest, RemoteNameRequestCancellationShouldDequeueNext) {

  auto promise = std::promise<void>{};

  auto future = promise.get_future();



  // start an outgoing request

  acl_scheduler_->EnqueueRemoteNameRequest(address1, emptyCallback(), impossibleCallback());

  // enqueue a second one

  acl_scheduler_->EnqueueRemoteNameRequest(address2, promiseCallback(std::move(promise)), impossibleCallback());



  // we should still be queued

  EXPECT_THAT(future.wait_for(timeout), std::future_status::timeout);



  // the first request is cancelled

  acl_scheduler_->CancelRemoteNameRequest(address1, emptyCallback());



  // we should still remain queued while we wait for the cancel to complete

  EXPECT_THAT(future.wait_for(timeout), std::future_status::timeout);



  // the cancel completes

  acl_scheduler_->ReportRemoteNameRequestCompletion(address1);



  // so the second request should now have started

  EXPECT_THAT(future, IsSet());

}



}  // namespace

}  // namespace acl_manager

}  // namespace hci

system/gd/rust/common/src/init_flags.rs

+1 −0
Original line number Diff line number Diff line
@@ -177,6 +177,7 @@ init_flags!( 
        gd_core,

        gd_l2cap,

        gd_link_policy,

        gd_remote_name_request,

        gd_rust,

        gd_security,

        hci_adapter: i32,

system/gd/rust/shim/src/init_flags.rs

+1 −0
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ mod ffi { 
        fn gd_core_is_enabled() -> bool;

        fn gd_l2cap_is_enabled() -> bool;

        fn gd_link_policy_is_enabled() -> bool;

        fn gd_remote_name_request_is_enabled() -> bool;

        fn gd_rust_is_enabled() -> bool;

        fn gd_security_is_enabled() -> bool;

        fn get_hci_adapter() -> i32;