Wificond: Make Offload HAL service nonblocking

Offload HAL getService() is a blocking call which can cause a deadlock.
Replace it with non blocking call tryGetService() which returns null if
the service is not running instead of waiting on the service to start.
This implies that the client needs to be able to get a reference to the
service at a later time. The design choice is to use tryGetService() API
to get the handle when a disconnected mode PNO scan is requested if this
was not available before.

Bug: 62807929
Test: Unit test, sanity test
Change-Id: I344fd93c4afe2381c356e626e83311af49a5ad49

diff --git a/scanning/offload/offload_scan_manager.cpp b/scanning/offload/offload_scan_manager.cpp
index 5c73362..420c2fa 100644 (file)
--- a/scanning/offload/offload_scan_manager.cpp
+++ b/scanning/offload/offload_scan_manager.cpp
@@ -74,39 +74,64 @@ OffloadScanManager::OffloadScanManager(
     shared_ptr<OffloadScanCallbackInterface> callback)
     : wifi_offload_hal_(nullptr),
       wifi_offload_callback_(nullptr),
+      death_recipient_(nullptr),
       offload_status_(OffloadScanManager::kError),
+      service_available_(false),
+      offload_service_utils_(utils),
       offload_callback_handlers_(new OffloadCallbackHandlersImpl(this)),
       event_callback_(callback) {
-  auto offload_scan_utils = utils.lock();
-  wifi_offload_hal_ = offload_scan_utils->GetOffloadService();
+  if (InitService()) {
+    offload_status_ = OffloadScanManager::kNoError;
+  }
+}
+
+bool OffloadScanManager::InitService() {
+  wifi_offload_hal_ = offload_service_utils_.lock()->GetOffloadService();
   if (wifi_offload_hal_ == nullptr) {
     LOG(ERROR) << "No Offload Service available";
-    return;
+    return false;
   }
 
-  death_recipient_ = offload_scan_utils->GetOffloadDeathRecipient(
+  death_recipient_ = offload_service_utils_.lock()->GetOffloadDeathRecipient(
       std::bind(&OffloadScanManager::OnObjectDeath, this, _1));
   uint64_t cookie = reinterpret_cast<uint64_t>(wifi_offload_hal_.get());
+
   auto link_to_death_status =
       wifi_offload_hal_->linkToDeath(death_recipient_, cookie);
   if (!link_to_death_status.isOk()) {
     LOG(ERROR) << "Unable to register death handler "
                << link_to_death_status.description();
-    return;
+    return false;
   }
 
-  wifi_offload_callback_ =
-      offload_scan_utils->GetOffloadCallback(offload_callback_handlers_.get());
+  wifi_offload_callback_ = offload_service_utils_.lock()->GetOffloadCallback(
+      offload_callback_handlers_.get());
   if (wifi_offload_callback_ == nullptr) {
     LOG(ERROR) << "Invalid Offload callback object";
-    return;
+    return false;
+  }
+
+  auto set_callback_status =
+      wifi_offload_hal_->setEventCallback(wifi_offload_callback_);
+  if (!set_callback_status.isOk()) {
+    LOG(ERROR) << "Unable to set event callback for Offload HAL";
+    return false;
+  }
+
+  service_available_ = true;
+  return true;
+}
+
+bool OffloadScanManager::InitServiceIfNeeded() {
+  if (!service_available_) {
+    return InitService();
   }
-  wifi_offload_hal_->setEventCallback(wifi_offload_callback_);
-  offload_status_ = OffloadScanManager::kNoError;
+  return true;
 }
 
 bool OffloadScanManager::stopScan(OffloadScanManager::ReasonCode* reason_code) {
-  if (getOffloadStatus() != OffloadScanManager::kNoError) {
+  if (!InitServiceIfNeeded() ||
+      (getOffloadStatus() != OffloadScanManager::kNoError)) {
     *reason_code = OffloadScanManager::kNotAvailable;
     return false;
   }
@@ -160,7 +185,8 @@ bool OffloadScanManager::startScan(
     const vector<vector<uint8_t>>& match_ssids,
     const vector<uint8_t>& match_security, const vector<uint32_t>& freqs,
     OffloadScanManager::ReasonCode* reason_code) {
-  if (getOffloadStatus() != OffloadScanManager::kNoError) {
+  if (!InitServiceIfNeeded() ||
+      getOffloadStatus() != OffloadScanManager::kNoError) {
     *reason_code = OffloadScanManager::kNotAvailable;
     LOG(WARNING) << "Offload HAL scans are not available";
     return false;
@@ -206,7 +232,7 @@ bool OffloadScanManager::SubscribeScanResults(
 }
 
 OffloadScanManager::StatusCode OffloadScanManager::getOffloadStatus() const {
-  if (wifi_offload_hal_ == nullptr) {
+  if (!service_available_) {
     return OffloadScanManager::kNoService;
   }
   return offload_status_;
@@ -221,6 +247,10 @@ bool OffloadScanManager::getScanResults(
 }
 
 bool OffloadScanManager::getScanStats(NativeScanStats* native_scan_stats) {
+  if (!InitServiceIfNeeded()) {
+    LOG(ERROR) << "Offload HAL service unavailable";
+    return false;
+  }
   if (getOffloadStatus() != OffloadScanManager::kNoError) {
     LOG(WARNING) << "Unable to get scan stats due to Wifi Offload HAL error";
     return false;
@@ -280,6 +310,7 @@ void OffloadScanManager::ReportError(const OffloadStatus& status) {
 
 void OffloadScanManager::OnObjectDeath(uint64_t cookie) {
   if (wifi_offload_hal_ == reinterpret_cast<IOffload*>(cookie)) {
+    LOG(ERROR) << "Death Notification for Wifi Offload HAL";
     wifi_offload_hal_.clear();
     if (event_callback_ != nullptr) {
       event_callback_->OnOffloadError(
@@ -288,8 +319,8 @@ void OffloadScanManager::OnObjectDeath(uint64_t cookie) {
       LOG(WARNING)
           << "No callback to report Offload HAL Binder death to wificond";
     }
-
-    LOG(ERROR) << "Death Notification for Wifi Offload HAL";
+    service_available_ = false;
+    death_recipient_.clear();
   }
 }
 

diff --git a/scanning/offload/offload_scan_manager.h b/scanning/offload/offload_scan_manager.h
index c928ba9..9cc3d47 100644 (file)
--- a/scanning/offload/offload_scan_manager.h
+++ b/scanning/offload/offload_scan_manager.h
@@ -132,6 +132,9 @@ class OffloadScanManager {
   bool ConfigureScans(android::hardware::wifi::offload::V1_0::ScanParam,
                       android::hardware::wifi::offload::V1_0::ScanFilter,
                       OffloadScanManager::ReasonCode* reason_code);
+  bool InitServiceIfNeeded();
+  bool InitService();
+
   /* Handle binder death */
   void OnObjectDeath(uint64_t /* cookie */);
 
@@ -142,7 +145,9 @@ class OffloadScanManager {
   StatusCode offload_status_;
   std::vector<::com::android::server::wifi::wificond::NativeScanResult>
       cached_scan_results_;
+  bool service_available_;
 
+  const std::weak_ptr<OffloadServiceUtils> offload_service_utils_;
   const std::unique_ptr<OffloadCallbackHandlersImpl> offload_callback_handlers_;
   std::shared_ptr<OffloadScanCallbackInterface> event_callback_;
 

diff --git a/scanning/offload/offload_service_utils.cpp b/scanning/offload/offload_service_utils.cpp
index da26b71..a4a2474 100644 (file)
--- a/scanning/offload/offload_service_utils.cpp
+++ b/scanning/offload/offload_service_utils.cpp
@@ -25,7 +25,7 @@ namespace android {
 namespace wificond {
 
 android::sp<IOffload> OffloadServiceUtils::GetOffloadService() {
-  return IOffload::getService();
+  return IOffload::tryGetService();
 }
 
 android::sp<OffloadCallback> OffloadServiceUtils::GetOffloadCallback(

diff --git a/tests/offload_scan_manager_test.cpp b/tests/offload_scan_manager_test.cpp
index 9c612ae..9e586cb 100644 (file)
--- a/tests/offload_scan_manager_test.cpp
+++ b/tests/offload_scan_manager_test.cpp
@@ -252,7 +252,7 @@ TEST_F(OffloadScanManagerTest, StartScanTestWhenServiceIsOk) {
  * Offload HAL when service is not available
  */
 TEST_F(OffloadScanManagerTest, StartScanTestWhenServiceIsNotAvailable) {
-  EXPECT_CALL(*mock_offload_service_utils_, GetOffloadService());
+  EXPECT_CALL(*mock_offload_service_utils_, GetOffloadService()).Times(2);
   ON_CALL(*mock_offload_service_utils_, GetOffloadService())
       .WillByDefault(testing::Return(nullptr));
   offload_scan_manager_.reset(new OffloadScanManager(