Merge changes Ieee26013,I275bba11,I7e7258e7,I88111181 am: 3646298ab6 am: b853615e6e am: f404487631

am: 4ffdf9c3a0

Change-Id: Ib9e28840ac6a6c7b4667ff037e80f0219f8307b8

wifi/1.0/default/Android.mk

@@ -19,6 +19,7 @@ LOCAL_MODULE_RELATIVE_PATH := hw
 LOCAL_CPPFLAGS := -Wall -Werror -Wextra
 LOCAL_SRC_FILES := \
     hidl_struct_util.cpp \
+    hidl_sync_util.cpp \
     service.cpp \
     wifi.cpp \
     wifi_ap_iface.cpp \

wifi/1.0/default/THREADING.README

@@ -0,0 +1,35 @@
+Vendor HAL Threading Model
+==========================
+The vendor HAL service has two threads:
+1. HIDL thread: This is the main thread which processes all the incoming HIDL
+RPC's.
+2. Legacy HAL event loop thread: This is the thread forked off for processing
+the legacy HAL event loop (wifi_event_loop()). This thread is used to process
+any asynchronous netlink events posted by the driver. Any asynchronous
+callbacks passed to the legacy HAL API's are invoked on this thread.
+
+Synchronization Concerns
+========================
+wifi_legacy_hal.cpp has a bunch of global "C" style functions to handle the
+legacy callbacks. Each of these "C" style function invokes a corresponding
+"std::function" version of the callback which does the actual processing.
+The variables holding these "std::function" callbacks are reset from the HIDL
+thread when they are no longer used. For example: stopGscan() will reset the
+corresponding "on_gscan_*" callback variables which were set when startGscan()
+was invoked. This is not thread safe since these callback variables are
+accesed from the legacy hal event loop thread as well.
+
+Synchronization Solution
+========================
+Adding a global lock seems to be the most trivial solution to the problem.
+a) All of the asynchronous "C" style callbacks will acquire the global lock
+before invoking the corresponding "std::function" callback variables.
+b) All of the HIDL methods will also acquire the global lock before processing
+(in hidl_return_util::validateAndCall()).
+
+Note: It's important that we only acquire the global lock for asynchronous
+callbacks, because there is no guarantee (or documentation to clarify) that the
+synchronous callbacks are invoked on the same invocation thread. If that is not
+the case in some implementation, we will end up deadlocking the system since the
+HIDL thread would have acquired the global lock which is needed by the
+synchronous callback executed on the legacy hal event loop thread.

wifi/1.0/default/hidl_return_util.h

@@ -17,6 +17,7 @@
 #ifndef HIDL_RETURN_UTIL_H_
 #define HIDL_RETURN_UTIL_H_
 
+#include "hidl_sync_util.h"
 #include "wifi_status_util.h"
 
 namespace android {
@@ -44,6 +45,7 @@ Return<void> validateAndCall(
     WorkFuncT&& work,
     const std::function<void(const WifiStatus&)>& hidl_cb,
     Args&&... args) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (obj->isValid()) {
     hidl_cb((obj->*work)(std::forward<Args>(args)...));
   } else {
@@ -61,6 +63,7 @@ Return<void> validateAndCall(
     WorkFuncT&& work,
     const std::function<void(const WifiStatus&, ReturnT)>& hidl_cb,
     Args&&... args) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (obj->isValid()) {
     const auto& ret_pair = (obj->*work)(std::forward<Args>(args)...);
     const WifiStatus& status = std::get<0>(ret_pair);
@@ -86,6 +89,7 @@ Return<void> validateAndCall(
     WorkFuncT&& work,
     const std::function<void(const WifiStatus&, ReturnT1, ReturnT2)>& hidl_cb,
     Args&&... args) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (obj->isValid()) {
     const auto& ret_tuple = (obj->*work)(std::forward<Args>(args)...);
     const WifiStatus& status = std::get<0>(ret_tuple);

wifi/1.0/default/hidl_sync_util.cpp

@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "hidl_sync_util.h"
+
+namespace {
+std::recursive_mutex g_mutex;
+}  // namespace
+
+namespace android {
+namespace hardware {
+namespace wifi {
+namespace V1_0 {
+namespace implementation {
+namespace hidl_sync_util {
+
+std::unique_lock<std::recursive_mutex> acquireGlobalLock() {
+  return std::unique_lock<std::recursive_mutex>{g_mutex};
+}
+
+}  // namespace hidl_sync_util
+}  // namespace implementation
+}  // namespace V1_0
+}  // namespace wifi
+}  // namespace hardware
+}  // namespace android

wifi/1.0/default/hidl_sync_util.h

@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef HIDL_SYNC_UTIL_H_
+#define HIDL_SYNC_UTIL_H_
+
+#include <mutex>
+
+// Utility that provides a global lock to synchronize access between
+// the HIDL thread and the legacy HAL's event loop.
+namespace android {
+namespace hardware {
+namespace wifi {
+namespace V1_0 {
+namespace implementation {
+namespace hidl_sync_util {
+std::unique_lock<std::recursive_mutex> acquireGlobalLock();
+}  // namespace hidl_sync_util
+}  // namespace implementation
+}  // namespace V1_0
+}  // namespace wifi
+}  // namespace hardware
+}  // namespace android
+#endif  // HIDL_SYNC_UTIL_H_

wifi/1.0/default/service.cpp

@@ -27,7 +27,7 @@ using android::hardware::joinRpcThreadpool;
 int main(int /*argc*/, char** argv) {
   android::base::InitLogging(argv,
                              android::base::LogdLogger(android::base::SYSTEM));
-  LOG(INFO) << "wifi_hal_legacy is starting up...";
+  LOG(INFO) << "Wifi Hal is starting up...";
 
   configureRpcThreadpool(1, true /* callerWillJoin */);
 
@@ -39,6 +39,6 @@ int main(int /*argc*/, char** argv) {
 
   joinRpcThreadpool();
 
-  LOG(INFO) << "wifi_hal_legacy is terminating...";
+  LOG(INFO) << "Wifi Hal is terminating...";
   return 0;
 }

wifi/1.0/default/wifi_chip.cpp

@@ -415,12 +415,16 @@ WifiStatus WifiChip::configureChipInternal(ChipModeId mode_id) {
   WifiStatus status = handleChipConfiguration(mode_id);
   if (status.code != WifiStatusCode::SUCCESS) {
     for (const auto& callback : event_callbacks_) {
-      callback->onChipReconfigureFailure(status);
+      if (!callback->onChipReconfigureFailure(status).isOk()) {
+        LOG(ERROR) << "Failed to invoke onChipReconfigureFailure callback";
+      }
     }
     return status;
   }
   for (const auto& callback : event_callbacks_) {
-    callback->onChipReconfigured(mode_id);
+    if (!callback->onChipReconfigured(mode_id).isOk()) {
+      LOG(ERROR) << "Failed to invoke onChipReconfigured callback";
+    }
   }
   current_mode_id_ = mode_id;
   return status;
@@ -500,7 +504,9 @@ std::pair<WifiStatus, sp<IWifiApIface>> WifiChip::createApIfaceInternal() {
   std::string ifname = legacy_hal_.lock()->getApIfaceName();
   ap_iface_ = new WifiApIface(ifname, legacy_hal_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceAdded(IfaceType::AP, ifname);
+    if (!callback->onIfaceAdded(IfaceType::AP, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceAdded callback";
+    }
   }
   return {createWifiStatus(WifiStatusCode::SUCCESS), ap_iface_};
 }
@@ -528,7 +534,9 @@ WifiStatus WifiChip::removeApIfaceInternal(const std::string& ifname) {
   }
   invalidateAndClear(ap_iface_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceRemoved(IfaceType::AP, ifname);
+    if (!callback->onIfaceRemoved(IfaceType::AP, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceRemoved callback";
+    }
   }
   return createWifiStatus(WifiStatusCode::SUCCESS);
 }
@@ -542,7 +550,9 @@ std::pair<WifiStatus, sp<IWifiNanIface>> WifiChip::createNanIfaceInternal() {
   std::string ifname = legacy_hal_.lock()->getNanIfaceName();
   nan_iface_ = new WifiNanIface(ifname, legacy_hal_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceAdded(IfaceType::NAN, ifname);
+    if (!callback->onIfaceAdded(IfaceType::NAN, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceAdded callback";
+    }
   }
   return {createWifiStatus(WifiStatusCode::SUCCESS), nan_iface_};
 }
@@ -570,7 +580,9 @@ WifiStatus WifiChip::removeNanIfaceInternal(const std::string& ifname) {
   }
   invalidateAndClear(nan_iface_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceRemoved(IfaceType::NAN, ifname);
+    if (!callback->onIfaceRemoved(IfaceType::NAN, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceAdded callback";
+    }
   }
   return createWifiStatus(WifiStatusCode::SUCCESS);
 }
@@ -584,7 +596,9 @@ std::pair<WifiStatus, sp<IWifiP2pIface>> WifiChip::createP2pIfaceInternal() {
   std::string ifname = legacy_hal_.lock()->getP2pIfaceName();
   p2p_iface_ = new WifiP2pIface(ifname, legacy_hal_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceAdded(IfaceType::P2P, ifname);
+    if (!callback->onIfaceAdded(IfaceType::P2P, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceAdded callback";
+    }
   }
   return {createWifiStatus(WifiStatusCode::SUCCESS), p2p_iface_};
 }
@@ -612,7 +626,9 @@ WifiStatus WifiChip::removeP2pIfaceInternal(const std::string& ifname) {
   }
   invalidateAndClear(p2p_iface_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceRemoved(IfaceType::P2P, ifname);
+    if (!callback->onIfaceRemoved(IfaceType::P2P, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceRemoved callback";
+    }
   }
   return createWifiStatus(WifiStatusCode::SUCCESS);
 }
@@ -624,7 +640,9 @@ std::pair<WifiStatus, sp<IWifiStaIface>> WifiChip::createStaIfaceInternal() {
   std::string ifname = legacy_hal_.lock()->getStaIfaceName();
   sta_iface_ = new WifiStaIface(ifname, legacy_hal_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceAdded(IfaceType::STA, ifname);
+    if (!callback->onIfaceAdded(IfaceType::STA, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceAdded callback";
+    }
   }
   return {createWifiStatus(WifiStatusCode::SUCCESS), sta_iface_};
 }
@@ -652,7 +670,9 @@ WifiStatus WifiChip::removeStaIfaceInternal(const std::string& ifname) {
   }
   invalidateAndClear(sta_iface_);
   for (const auto& callback : event_callbacks_) {
-    callback->onIfaceRemoved(IfaceType::STA, ifname);
+    if (!callback->onIfaceRemoved(IfaceType::STA, ifname).isOk()) {
+      LOG(ERROR) << "Failed to invoke onIfaceRemoved callback";
+    }
   }
   return createWifiStatus(WifiStatusCode::SUCCESS);
 }
@@ -743,7 +763,9 @@ WifiStatus WifiChip::enableDebugErrorAlertsInternal(bool enable) {
         return;
       }
       for (const auto& callback : shared_ptr_this->getEventCallbacks()) {
-        callback->onDebugErrorAlert(error_code, debug_data);
+        if (!callback->onDebugErrorAlert(error_code, debug_data).isOk()) {
+          LOG(ERROR) << "Failed to invoke onDebugErrorAlert callback";
+        }
       }
     };
     legacy_status = legacy_hal_.lock()->registerErrorAlertCallbackHandler(
@@ -806,7 +828,10 @@ WifiStatus WifiChip::registerDebugRingBufferCallback() {
       return;
     }
     for (const auto& callback : shared_ptr_this->getEventCallbacks()) {
-      callback->onDebugRingBufferDataAvailable(hidl_status, data);
+      if (!callback->onDebugRingBufferDataAvailable(hidl_status, data).isOk()) {
+        LOG(ERROR) << "Failed to invoke onDebugRingBufferDataAvailable"
+                   << " callback";
+      }
     }
   };
   legacy_hal::wifi_error legacy_status =

wifi/1.0/default/wifi_legacy_hal.cpp

@@ -19,6 +19,7 @@
 #include <android-base/logging.h>
 #include <cutils/properties.h>
 
+#include "hidl_sync_util.h"
 #include "wifi_legacy_hal.h"
 #include "wifi_legacy_hal_stubs.h"
 
@@ -54,7 +55,8 @@ namespace legacy_hal {
 // std::function methods to be invoked.
 // Callback to be invoked once |stop| is complete.
 std::function<void(wifi_handle handle)> on_stop_complete_internal_callback;
-void onStopComplete(wifi_handle handle) {
+void onAsyncStopComplete(wifi_handle handle) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_stop_complete_internal_callback) {
     on_stop_complete_internal_callback(handle);
   }
@@ -62,7 +64,7 @@ void onStopComplete(wifi_handle handle) {
 
 // Callback to be invoked for driver dump.
 std::function<void(char*, int)> on_driver_memory_dump_internal_callback;
-void onDriverMemoryDump(char* buffer, int buffer_size) {
+void onSyncDriverMemoryDump(char* buffer, int buffer_size) {
   if (on_driver_memory_dump_internal_callback) {
     on_driver_memory_dump_internal_callback(buffer, buffer_size);
   }
@@ -70,7 +72,7 @@ void onDriverMemoryDump(char* buffer, int buffer_size) {
 
 // Callback to be invoked for firmware dump.
 std::function<void(char*, int)> on_firmware_memory_dump_internal_callback;
-void onFirmwareMemoryDump(char* buffer, int buffer_size) {
+void onSyncFirmwareMemoryDump(char* buffer, int buffer_size) {
   if (on_firmware_memory_dump_internal_callback) {
     on_firmware_memory_dump_internal_callback(buffer, buffer_size);
   }
@@ -79,7 +81,8 @@ void onFirmwareMemoryDump(char* buffer, int buffer_size) {
 // Callback to be invoked for Gscan events.
 std::function<void(wifi_request_id, wifi_scan_event)>
     on_gscan_event_internal_callback;
-void onGscanEvent(wifi_request_id id, wifi_scan_event event) {
+void onAsyncGscanEvent(wifi_request_id id, wifi_scan_event event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_gscan_event_internal_callback) {
     on_gscan_event_internal_callback(id, event);
   }
@@ -88,9 +91,10 @@ void onGscanEvent(wifi_request_id id, wifi_scan_event event) {
 // Callback to be invoked for Gscan full results.
 std::function<void(wifi_request_id, wifi_scan_result*, uint32_t)>
     on_gscan_full_result_internal_callback;
-void onGscanFullResult(wifi_request_id id,
-                       wifi_scan_result* result,
-                       uint32_t buckets_scanned) {
+void onAsyncGscanFullResult(wifi_request_id id,
+                            wifi_scan_result* result,
+                            uint32_t buckets_scanned) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_gscan_full_result_internal_callback) {
     on_gscan_full_result_internal_callback(id, result, buckets_scanned);
   }
@@ -99,10 +103,10 @@ void onGscanFullResult(wifi_request_id id,
 // Callback to be invoked for link layer stats results.
 std::function<void((wifi_request_id, wifi_iface_stat*, int, wifi_radio_stat*))>
     on_link_layer_stats_result_internal_callback;
-void onLinkLayerStatsResult(wifi_request_id id,
-                            wifi_iface_stat* iface_stat,
-                            int num_radios,
-                            wifi_radio_stat* radio_stat) {
+void onSyncLinkLayerStatsResult(wifi_request_id id,
+                                wifi_iface_stat* iface_stat,
+                                int num_radios,
+                                wifi_radio_stat* radio_stat) {
   if (on_link_layer_stats_result_internal_callback) {
     on_link_layer_stats_result_internal_callback(
         id, iface_stat, num_radios, radio_stat);
@@ -112,7 +116,10 @@ void onLinkLayerStatsResult(wifi_request_id id,
 // Callback to be invoked for rssi threshold breach.
 std::function<void((wifi_request_id, uint8_t*, int8_t))>
     on_rssi_threshold_breached_internal_callback;
-void onRssiThresholdBreached(wifi_request_id id, uint8_t* bssid, int8_t rssi) {
+void onAsyncRssiThresholdBreached(wifi_request_id id,
+                                  uint8_t* bssid,
+                                  int8_t rssi) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_rssi_threshold_breached_internal_callback) {
     on_rssi_threshold_breached_internal_callback(id, bssid, rssi);
   }
@@ -121,10 +128,11 @@ void onRssiThresholdBreached(wifi_request_id id, uint8_t* bssid, int8_t rssi) {
 // Callback to be invoked for ring buffer data indication.
 std::function<void(char*, char*, int, wifi_ring_buffer_status*)>
     on_ring_buffer_data_internal_callback;
-void onRingBufferData(char* ring_name,
-                      char* buffer,
-                      int buffer_size,
-                      wifi_ring_buffer_status* status) {
+void onAsyncRingBufferData(char* ring_name,
+                           char* buffer,
+                           int buffer_size,
+                           wifi_ring_buffer_status* status) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_ring_buffer_data_internal_callback) {
     on_ring_buffer_data_internal_callback(
         ring_name, buffer, buffer_size, status);
@@ -134,10 +142,11 @@ void onRingBufferData(char* ring_name,
 // Callback to be invoked for error alert indication.
 std::function<void(wifi_request_id, char*, int, int)>
     on_error_alert_internal_callback;
-void onErrorAlert(wifi_request_id id,
-                  char* buffer,
-                  int buffer_size,
-                  int err_code) {
+void onAsyncErrorAlert(wifi_request_id id,
+                       char* buffer,
+                       int buffer_size,
+                       int err_code) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_error_alert_internal_callback) {
     on_error_alert_internal_callback(id, buffer, buffer_size, err_code);
   }
@@ -147,9 +156,10 @@ void onErrorAlert(wifi_request_id id,
 std::function<void(
     wifi_request_id, unsigned num_results, wifi_rtt_result* rtt_results[])>
     on_rtt_results_internal_callback;
-void onRttResults(wifi_request_id id,
-                  unsigned num_results,
-                  wifi_rtt_result* rtt_results[]) {
+void onAsyncRttResults(wifi_request_id id,
+                       unsigned num_results,
+                       wifi_rtt_result* rtt_results[]) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_rtt_results_internal_callback) {
     on_rtt_results_internal_callback(id, num_results, rtt_results);
   }
@@ -161,7 +171,8 @@ void onRttResults(wifi_request_id id,
 // So, handle all of them here directly to avoid adding an unnecessary layer.
 std::function<void(transaction_id, const NanResponseMsg&)>
     on_nan_notify_response_user_callback;
-void onNanNotifyResponse(transaction_id id, NanResponseMsg* msg) {
+void onAysncNanNotifyResponse(transaction_id id, NanResponseMsg* msg) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_notify_response_user_callback && msg) {
     on_nan_notify_response_user_callback(id, *msg);
   }
@@ -169,14 +180,16 @@ void onNanNotifyResponse(transaction_id id, NanResponseMsg* msg) {
 
 std::function<void(const NanPublishTerminatedInd&)>
     on_nan_event_publish_terminated_user_callback;
-void onNanEventPublishTerminated(NanPublishTerminatedInd* event) {
+void onAysncNanEventPublishTerminated(NanPublishTerminatedInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_publish_terminated_user_callback && event) {
     on_nan_event_publish_terminated_user_callback(*event);
   }
 }
 
 std::function<void(const NanMatchInd&)> on_nan_event_match_user_callback;
-void onNanEventMatch(NanMatchInd* event) {
+void onAysncNanEventMatch(NanMatchInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_match_user_callback && event) {
     on_nan_event_match_user_callback(*event);
   }
@@ -184,7 +197,8 @@ void onNanEventMatch(NanMatchInd* event) {
 
 std::function<void(const NanMatchExpiredInd&)>
     on_nan_event_match_expired_user_callback;
-void onNanEventMatchExpired(NanMatchExpiredInd* event) {
+void onAysncNanEventMatchExpired(NanMatchExpiredInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_match_expired_user_callback && event) {
     on_nan_event_match_expired_user_callback(*event);
   }
@@ -192,14 +206,16 @@ void onNanEventMatchExpired(NanMatchExpiredInd* event) {
 
 std::function<void(const NanSubscribeTerminatedInd&)>
     on_nan_event_subscribe_terminated_user_callback;
-void onNanEventSubscribeTerminated(NanSubscribeTerminatedInd* event) {
+void onAysncNanEventSubscribeTerminated(NanSubscribeTerminatedInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_subscribe_terminated_user_callback && event) {
     on_nan_event_subscribe_terminated_user_callback(*event);
   }
 }
 
 std::function<void(const NanFollowupInd&)> on_nan_event_followup_user_callback;
-void onNanEventFollowup(NanFollowupInd* event) {
+void onAysncNanEventFollowup(NanFollowupInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_followup_user_callback && event) {
     on_nan_event_followup_user_callback(*event);
   }
@@ -207,21 +223,24 @@ void onNanEventFollowup(NanFollowupInd* event) {
 
 std::function<void(const NanDiscEngEventInd&)>
     on_nan_event_disc_eng_event_user_callback;
-void onNanEventDiscEngEvent(NanDiscEngEventInd* event) {
+void onAysncNanEventDiscEngEvent(NanDiscEngEventInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_disc_eng_event_user_callback && event) {
     on_nan_event_disc_eng_event_user_callback(*event);
   }
 }
 
 std::function<void(const NanDisabledInd&)> on_nan_event_disabled_user_callback;
-void onNanEventDisabled(NanDisabledInd* event) {
+void onAysncNanEventDisabled(NanDisabledInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_disabled_user_callback && event) {
     on_nan_event_disabled_user_callback(*event);
   }
 }
 
 std::function<void(const NanTCAInd&)> on_nan_event_tca_user_callback;
-void onNanEventTca(NanTCAInd* event) {
+void onAysncNanEventTca(NanTCAInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_tca_user_callback && event) {
     on_nan_event_tca_user_callback(*event);
   }
@@ -229,7 +248,8 @@ void onNanEventTca(NanTCAInd* event) {
 
 std::function<void(const NanBeaconSdfPayloadInd&)>
     on_nan_event_beacon_sdf_payload_user_callback;
-void onNanEventBeaconSdfPayload(NanBeaconSdfPayloadInd* event) {
+void onAysncNanEventBeaconSdfPayload(NanBeaconSdfPayloadInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_beacon_sdf_payload_user_callback && event) {
     on_nan_event_beacon_sdf_payload_user_callback(*event);
   }
@@ -237,14 +257,16 @@ void onNanEventBeaconSdfPayload(NanBeaconSdfPayloadInd* event) {
 
 std::function<void(const NanDataPathRequestInd&)>
     on_nan_event_data_path_request_user_callback;
-void onNanEventDataPathRequest(NanDataPathRequestInd* event) {
+void onAysncNanEventDataPathRequest(NanDataPathRequestInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_data_path_request_user_callback && event) {
     on_nan_event_data_path_request_user_callback(*event);
   }
 }
 std::function<void(const NanDataPathConfirmInd&)>
     on_nan_event_data_path_confirm_user_callback;
-void onNanEventDataPathConfirm(NanDataPathConfirmInd* event) {
+void onAysncNanEventDataPathConfirm(NanDataPathConfirmInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_data_path_confirm_user_callback && event) {
     on_nan_event_data_path_confirm_user_callback(*event);
   }
@@ -252,7 +274,8 @@ void onNanEventDataPathConfirm(NanDataPathConfirmInd* event) {
 
 std::function<void(const NanDataPathEndInd&)>
     on_nan_event_data_path_end_user_callback;
-void onNanEventDataPathEnd(NanDataPathEndInd* event) {
+void onAysncNanEventDataPathEnd(NanDataPathEndInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_data_path_end_user_callback && event) {
     on_nan_event_data_path_end_user_callback(*event);
   }
@@ -260,7 +283,8 @@ void onNanEventDataPathEnd(NanDataPathEndInd* event) {
 
 std::function<void(const NanTransmitFollowupInd&)>
     on_nan_event_transmit_follow_up_user_callback;
-void onNanEventTransmitFollowUp(NanTransmitFollowupInd* event) {
+void onAysncNanEventTransmitFollowUp(NanTransmitFollowupInd* event) {
+  const auto lock = hidl_sync_util::acquireGlobalLock();
   if (on_nan_event_transmit_follow_up_user_callback && event) {
     on_nan_event_transmit_follow_up_user_callback(*event);
   }
@@ -326,7 +350,8 @@ wifi_error WifiLegacyHal::stop(
     return WIFI_SUCCESS;
   }
   LOG(DEBUG) << "Stopping legacy HAL";
-  on_stop_complete_internal_callback = [&](wifi_handle handle) {
+  on_stop_complete_internal_callback = [on_stop_complete_user_callback,
+                                        this](wifi_handle handle) {
     CHECK_EQ(global_handle_, handle) << "Handle mismatch";
     // Invalidate all the internal pointers now that the HAL is
     // stopped.
@@ -335,7 +360,7 @@ wifi_error WifiLegacyHal::stop(
     on_stop_complete_user_callback();
   };
   awaiting_event_loop_termination_ = true;
-  global_func_table_.wifi_cleanup(global_handle_, onStopComplete);
+  global_func_table_.wifi_cleanup(global_handle_, onAsyncStopComplete);
   LOG(DEBUG) << "Legacy HAL stop complete";
   is_started_ = false;
   return WIFI_SUCCESS;
@@ -391,7 +416,7 @@ WifiLegacyHal::requestDriverMemoryDump() {
                        reinterpret_cast<uint8_t*>(buffer) + buffer_size);
   };
   wifi_error status = global_func_table_.wifi_get_driver_memory_dump(
-      wlan_interface_handle_, {onDriverMemoryDump});
+      wlan_interface_handle_, {onSyncDriverMemoryDump});
   on_driver_memory_dump_internal_callback = nullptr;
   return {status, std::move(driver_dump)};
 }
@@ -406,7 +431,7 @@ WifiLegacyHal::requestFirmwareMemoryDump() {
                          reinterpret_cast<uint8_t*>(buffer) + buffer_size);
   };
   wifi_error status = global_func_table_.wifi_get_firmware_memory_dump(
-      wlan_interface_handle_, {onFirmwareMemoryDump});
+      wlan_interface_handle_, {onSyncFirmwareMemoryDump});
   on_firmware_memory_dump_internal_callback = nullptr;
   return {status, std::move(firmware_dump)};
 }
@@ -488,7 +513,8 @@ wifi_error WifiLegacyHal::startGscan(
     }
   };
 
-  wifi_scan_result_handler handler = {onGscanFullResult, onGscanEvent};
+  wifi_scan_result_handler handler = {onAsyncGscanFullResult,
+                                      onAsyncGscanEvent};
   wifi_error status = global_func_table_.wifi_start_gscan(
       id, wlan_interface_handle_, params, handler);
   if (status != WIFI_SUCCESS) {
@@ -584,7 +610,7 @@ std::pair<wifi_error, LinkLayerStats> WifiLegacyHal::getLinkLayerStats() {
   };
 
   wifi_error status = global_func_table_.wifi_get_link_stats(
-      0, wlan_interface_handle_, {onLinkLayerStatsResult});
+      0, wlan_interface_handle_, {onSyncLinkLayerStatsResult});
   on_link_layer_stats_result_internal_callback = nullptr;
   return {status, link_stats};
 }
@@ -609,12 +635,12 @@ wifi_error WifiLegacyHal::startRssiMonitoring(
         std::copy(bssid_ptr, bssid_ptr + 6, std::begin(bssid_arr));
         on_threshold_breached_user_callback(id, bssid_arr, rssi);
       };
-  wifi_error status =
-      global_func_table_.wifi_start_rssi_monitoring(id,
-                                                    wlan_interface_handle_,
-                                                    max_rssi,
-                                                    min_rssi,
-                                                    {onRssiThresholdBreached});
+  wifi_error status = global_func_table_.wifi_start_rssi_monitoring(
+      id,
+      wlan_interface_handle_,
+      max_rssi,
+      min_rssi,
+      {onAsyncRssiThresholdBreached});
   if (status != WIFI_SUCCESS) {
     on_rssi_threshold_breached_internal_callback = nullptr;
   }
@@ -789,7 +815,7 @@ wifi_error WifiLegacyHal::registerRingBufferCallbackHandler(
     }
   };
   wifi_error status = global_func_table_.wifi_set_log_handler(
-      0, wlan_interface_handle_, {onRingBufferData});
+      0, wlan_interface_handle_, {onAsyncRingBufferData});
   if (status != WIFI_SUCCESS) {
     on_ring_buffer_data_internal_callback = nullptr;
   }
@@ -850,7 +876,7 @@ wifi_error WifiLegacyHal::registerErrorAlertCallbackHandler(
     }
   };
   wifi_error status = global_func_table_.wifi_set_alert_handler(
-      0, wlan_interface_handle_, {onErrorAlert});
+      0, wlan_interface_handle_, {onAsyncErrorAlert});
   if (status != WIFI_SUCCESS) {
     on_error_alert_internal_callback = nullptr;
   }
@@ -896,7 +922,7 @@ wifi_error WifiLegacyHal::startRttRangeRequest(
                                                 wlan_interface_handle_,
                                                 rtt_configs.size(),
                                                 rtt_configs_internal.data(),
-                                                {onRttResults});
+                                                {onAsyncRttResults});
   if (status != WIFI_SUCCESS) {
     on_rtt_results_internal_callback = nullptr;
   }
@@ -1000,20 +1026,20 @@ wifi_error WifiLegacyHal::nanRegisterCallbackHandlers(
 
   return global_func_table_.wifi_nan_register_handler(
       wlan_interface_handle_,
-      {onNanNotifyResponse,
-       onNanEventPublishTerminated,
-       onNanEventMatch,
-       onNanEventMatchExpired,
-       onNanEventSubscribeTerminated,
-       onNanEventFollowup,
-       onNanEventDiscEngEvent,
-       onNanEventDisabled,
-       onNanEventTca,
-       onNanEventBeaconSdfPayload,
-       onNanEventDataPathRequest,
-       onNanEventDataPathConfirm,
-       onNanEventDataPathEnd,
-       onNanEventTransmitFollowUp});
+      {onAysncNanNotifyResponse,
+       onAysncNanEventPublishTerminated,
+       onAysncNanEventMatch,
+       onAysncNanEventMatchExpired,
+       onAysncNanEventSubscribeTerminated,
+       onAysncNanEventFollowup,
+       onAysncNanEventDiscEngEvent,
+       onAysncNanEventDisabled,
+       onAysncNanEventTca,
+       onAysncNanEventBeaconSdfPayload,
+       onAysncNanEventDataPathRequest,
+       onAysncNanEventDataPathConfirm,
+       onAysncNanEventDataPathEnd,
+       onAysncNanEventTransmitFollowUp});
 }
 
 wifi_error WifiLegacyHal::nanEnableRequest(transaction_id id,

wifi/1.0/default/wifi_legacy_hal.h

@@ -124,6 +124,9 @@ using on_error_alert_callback =
 /**
  * Class that encapsulates all legacy HAL interactions.
  * This class manages the lifetime of the event loop thread used by legacy HAL.
+ *
+ * Note: aThere will only be a single instance of this class created in the Wifi
+ * object and will be valid for the lifetime of the process.
  */
 class WifiLegacyHal {
  public:

wifi/1.0/default/wifi_sta_iface.cpp

@@ -389,7 +389,9 @@ WifiStatus WifiStaIface::startBackgroundScanInternal(
           return;
         }
         for (const auto& callback : shared_ptr_this->getEventCallbacks()) {
-          callback->onBackgroundScanFailure(id);
+          if (!callback->onBackgroundScanFailure(id).isOk()) {
+            LOG(ERROR) << "Failed to invoke onBackgroundScanFailure callback";
+          }
         }
       };
   const auto& on_results_callback = [weak_ptr_this](
@@ -407,7 +409,9 @@ WifiStatus WifiStaIface::startBackgroundScanInternal(
       return;
     }
     for (const auto& callback : shared_ptr_this->getEventCallbacks()) {
-      callback->onBackgroundScanResults(id, hidl_scan_datas);
+      if (!callback->onBackgroundScanResults(id, hidl_scan_datas).isOk()) {
+        LOG(ERROR) << "Failed to invoke onBackgroundScanResults callback";
+      }
     }
   };
   const auto& on_full_result_callback = [weak_ptr_this](
@@ -426,7 +430,9 @@ WifiStatus WifiStaIface::startBackgroundScanInternal(
       return;
     }
     for (const auto& callback : shared_ptr_this->getEventCallbacks()) {
-      callback->onBackgroundFullScanResult(id, hidl_scan_result);
+      if (!callback->onBackgroundFullScanResult(id, hidl_scan_result).isOk()) {
+        LOG(ERROR) << "Failed to invoke onBackgroundFullScanResult callback";
+      }
     }
   };
   legacy_hal::wifi_error legacy_status =
@@ -486,7 +492,9 @@ WifiStatus WifiStaIface::startRssiMonitoringInternal(uint32_t cmd_id,
       return;
     }
     for (const auto& callback : shared_ptr_this->getEventCallbacks()) {
-      callback->onRssiThresholdBreached(id, bssid, rssi);
+      if (!callback->onRssiThresholdBreached(id, bssid, rssi).isOk()) {
+        LOG(ERROR) << "Failed to invoke onRssiThresholdBreached callback";
+      }
     }
   };
   legacy_hal::wifi_error legacy_status =

wifi/supplicant/1.0/ISupplicantP2pIfaceCallback.hal

@@ -194,43 +194,6 @@ interface ISupplicantP2pIfaceCallback {
    */
   oneway onInvitationResult(Bssid bssid, P2pStatusCode status);
 
-  /**
-   * Used to indicate a push-button request generated during provision discovery.
-   *
-   * @param p2pDeviceAddress P2P device address.
-   */
-  oneway onProvisionDiscoveryPbcRequest(MacAddress p2pDeviceAddress);
-
-  /**
-   * Used to indicate a push-button response generated during provision discovery.
-   *
-   * @param p2pDeviceAddress P2P device address.
-   */
-  oneway onProvisionDiscoveryPbcResponse(MacAddress p2pDeviceAddress);
-
-  /**
-   * Used to indicate the pin generated during provision discovery.
-   *
-   * @param p2pDeviceAddress P2P device address.
-   * @param generatedPin 8 digit pin generated.
-   */
-  oneway onProvisionDiscoveryShowPin(
-      MacAddress p2pDeviceAddress, string generatedPin);
-
-  /**
-   * Used to indicate that a pin needs to be entered during provision discovery.
-   *
-   * @param p2pDeviceAddress P2P device address.
-   */
-  oneway onProvisionDiscoveryEnterPin(MacAddress p2pDeviceAddress);
-
-  /**
-   * Used to indicate a provision discovery failure.
-   *
-   * @param p2pDeviceAddress P2P device address.
-   */
-  oneway onProvisionDiscoveryFailure(MacAddress p2pDeviceAddress);
-
   /**
    * Used to indicate the completion of a P2P provision discovery request.
    *

wifi/supplicant/1.0/ISupplicantStaIfaceCallback.hal

@@ -256,13 +256,6 @@ interface ISupplicantStaIfaceCallback {
                                     uint32_t reAuthDelayInSec,
                                     string url);
 
-  /**
-   * Used to indicate the connection to a new network on this iface.
-   *
-   * @param bssid BSSID of the AP to which we connected.
-   */
-  oneway onConnected(Bssid bssid);
-
   /**
    * Used to indicate the disconnection from the currently connected
    * network on this iface.
@@ -276,13 +269,6 @@ interface ISupplicantStaIfaceCallback {
   oneway onDisconnected(
           Bssid bssid, bool locallyGenerated, uint32_t reasonCode);
 
-  /**
-   * Used to indicate the completion of association to an AP.
-   *
-   * @param bssid BSSID of the corresponding AP.
-   */
-  oneway onAssociationCompleted(Bssid bssid);
-
   /**
    * Used to indicate an association rejection recieved from the AP
    * to which the connection is being attempted.