diff --git a/wifi/1.0/default/Android.mk b/wifi/1.0/default/Android.mk index 144c067b92..ac484b62c5 100644 --- a/wifi/1.0/default/Android.mk +++ b/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 \ diff --git a/wifi/1.0/default/THREADING.README b/wifi/1.0/default/THREADING.README new file mode 100644 index 0000000000..8366ca0201 --- /dev/null +++ b/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. diff --git a/wifi/1.0/default/hidl_return_util.h b/wifi/1.0/default/hidl_return_util.h index 2986165507..3f6364be3f 100644 --- a/wifi/1.0/default/hidl_return_util.h +++ b/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 validateAndCall( WorkFuncT&& work, const std::function& hidl_cb, Args&&... args) { + const auto lock = hidl_sync_util::acquireGlobalLock(); if (obj->isValid()) { hidl_cb((obj->*work)(std::forward(args)...)); } else { @@ -61,6 +63,7 @@ Return validateAndCall( WorkFuncT&& work, const std::function& hidl_cb, Args&&... args) { + const auto lock = hidl_sync_util::acquireGlobalLock(); if (obj->isValid()) { const auto& ret_pair = (obj->*work)(std::forward(args)...); const WifiStatus& status = std::get<0>(ret_pair); @@ -86,6 +89,7 @@ Return validateAndCall( WorkFuncT&& work, const std::function& hidl_cb, Args&&... args) { + const auto lock = hidl_sync_util::acquireGlobalLock(); if (obj->isValid()) { const auto& ret_tuple = (obj->*work)(std::forward(args)...); const WifiStatus& status = std::get<0>(ret_tuple); diff --git a/wifi/1.0/default/hidl_sync_util.cpp b/wifi/1.0/default/hidl_sync_util.cpp new file mode 100644 index 0000000000..7d47f2f7bd --- /dev/null +++ b/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 acquireGlobalLock() { + return std::unique_lock{g_mutex}; +} + +} // namespace hidl_sync_util +} // namespace implementation +} // namespace V1_0 +} // namespace wifi +} // namespace hardware +} // namespace android diff --git a/wifi/1.0/default/hidl_sync_util.h b/wifi/1.0/default/hidl_sync_util.h new file mode 100644 index 0000000000..6631e55bca --- /dev/null +++ b/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 + +// 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 acquireGlobalLock(); +} // namespace hidl_sync_util +} // namespace implementation +} // namespace V1_0 +} // namespace wifi +} // namespace hardware +} // namespace android +#endif // HIDL_SYNC_UTIL_H_ diff --git a/wifi/1.0/default/service.cpp b/wifi/1.0/default/service.cpp index 96d5c6f78e..059304e335 100644 --- a/wifi/1.0/default/service.cpp +++ b/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; } diff --git a/wifi/1.0/default/wifi_chip.cpp b/wifi/1.0/default/wifi_chip.cpp index e15178d824..0e2d54e653 100644 --- a/wifi/1.0/default/wifi_chip.cpp +++ b/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> 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> 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> 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> 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 = diff --git a/wifi/1.0/default/wifi_legacy_hal.cpp b/wifi/1.0/default/wifi_legacy_hal.cpp index b0b0f96336..cd89accf15 100644 --- a/wifi/1.0/default/wifi_legacy_hal.cpp +++ b/wifi/1.0/default/wifi_legacy_hal.cpp @@ -19,6 +19,7 @@ #include #include +#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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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(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(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 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, diff --git a/wifi/1.0/default/wifi_legacy_hal.h b/wifi/1.0/default/wifi_legacy_hal.h index dce4ed4b75..e65b79b034 100644 --- a/wifi/1.0/default/wifi_legacy_hal.h +++ b/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: diff --git a/wifi/1.0/default/wifi_sta_iface.cpp b/wifi/1.0/default/wifi_sta_iface.cpp index 6cc41db87d..6100334f9c 100644 --- a/wifi/1.0/default/wifi_sta_iface.cpp +++ b/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 = diff --git a/wifi/supplicant/1.0/ISupplicantP2pIfaceCallback.hal b/wifi/supplicant/1.0/ISupplicantP2pIfaceCallback.hal index ad4290b34e..b6ee57f9fd 100644 --- a/wifi/supplicant/1.0/ISupplicantP2pIfaceCallback.hal +++ b/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. * diff --git a/wifi/supplicant/1.0/ISupplicantStaIfaceCallback.hal b/wifi/supplicant/1.0/ISupplicantStaIfaceCallback.hal index c3ec060065..34237f0e6f 100644 --- a/wifi/supplicant/1.0/ISupplicantStaIfaceCallback.hal +++ b/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.