From d37341f1e57b91a5f8bf6fe85b99861717720e5f Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 31 Jan 2017 13:13:28 -0800 Subject: [PATCH] wifi(implementation): Callback death handler Add a new utility to handle callback death notifications. The new class HidlCallbackHandler will be used by all the HIDL interface objects to manage callbacks. Any dead clients will automatically removed from the cb list by the utility class. Bug: 34840719 Test: Compiles Test: Verified that the cbs are deleted on crashing the framework manually Change-Id: I0f7ba8b3ed717c2e8e8fbf744a2501d0ad2d48c8 --- wifi/1.0/default/hidl_callback_util.h | 123 ++++++++++++++++++++++++++ wifi/1.0/default/wifi.cpp | 13 +-- wifi/1.0/default/wifi.h | 3 +- wifi/1.0/default/wifi_chip.cpp | 31 +++---- wifi/1.0/default/wifi_chip.h | 6 +- wifi/1.0/default/wifi_nan_iface.cpp | 64 +++++++------- wifi/1.0/default/wifi_nan_iface.h | 6 +- wifi/1.0/default/wifi_sta_iface.cpp | 14 +-- wifi/1.0/default/wifi_sta_iface.h | 6 +- 9 files changed, 202 insertions(+), 64 deletions(-) create mode 100644 wifi/1.0/default/hidl_callback_util.h diff --git a/wifi/1.0/default/hidl_callback_util.h b/wifi/1.0/default/hidl_callback_util.h new file mode 100644 index 0000000000..a1c681987f --- /dev/null +++ b/wifi/1.0/default/hidl_callback_util.h @@ -0,0 +1,123 @@ +/* + * Copyright (C) 2017 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_CALLBACK_UTIL_H_ +#define HIDL_CALLBACK_UTIL_H_ + +#include + +#include + +namespace { +// Type of callback invoked by the death handler. +using on_death_cb_function = std::function; + +// Private class used to keep track of death of individual +// callbacks stored in HidlCallbackHandler. +template +class HidlDeathHandler : public android::hardware::hidl_death_recipient { + public: + HidlDeathHandler(const on_death_cb_function& user_cb_function) + : cb_function_(user_cb_function) {} + ~HidlDeathHandler() = default; + + // Death notification for callbacks. + void serviceDied( + uint64_t cookie, + const android::wp& /* who */) override { + cb_function_(cookie); + } + + private: + on_death_cb_function cb_function_; + + DISALLOW_COPY_AND_ASSIGN(HidlDeathHandler); +}; +} // namespace + +namespace android { +namespace hardware { +namespace wifi { +namespace V1_0 { +namespace implementation { +namespace hidl_callback_util { +template +// Provides a class to manage callbacks for the various HIDL interfaces and +// handle the death of the process hosting each callback. +class HidlCallbackHandler { + public: + HidlCallbackHandler() + : death_handler_(new HidlDeathHandler( + std::bind(&HidlCallbackHandler::onObjectDeath, + this, + std::placeholders::_1))) {} + ~HidlCallbackHandler() = default; + + bool addCallback(const sp& cb) { + // TODO(b/33818800): Can't compare proxies yet. So, use the cookie + // (callback proxy's raw pointer) to track the death of individual clients. + uint64_t cookie = reinterpret_cast(cb.get()); + if (cb_set_.find(cb) != cb_set_.end()) { + LOG(WARNING) << "Duplicate death notification registration"; + return true; + } + if (!cb->linkToDeath(death_handler_, cookie)) { + LOG(ERROR) << "Failed to register death notification"; + return false; + } + cb_set_.insert(cb); + return true; + } + + const std::set> getCallbacks() { + return cb_set_; + } + + // Death notification for callbacks. + void onObjectDeath(uint64_t cookie) { + CallbackType *cb = reinterpret_cast(cookie); + const auto& iter = cb_set_.find(cb); + if (iter == cb_set_.end()) { + LOG(ERROR) << "Unknown callback death notification received"; + return; + } + cb_set_.erase(iter); + LOG(DEBUG) << "Dead callback removed from list"; + } + + void invalidate() { + for (const sp& cb : cb_set_) { + if (!cb->unlinkToDeath(death_handler_)) { + LOG(ERROR) << "Failed to deregister death notification"; + } + } + cb_set_.clear(); + } + + private: + std::set> cb_set_; + sp> death_handler_; + + DISALLOW_COPY_AND_ASSIGN(HidlCallbackHandler); +}; + +} // namespace hidl_callback_util +} // namespace implementation +} // namespace V1_0 +} // namespace wifi +} // namespace hardware +} // namespace android +#endif // HIDL_CALLBACK_UTIL_H_ diff --git a/wifi/1.0/default/wifi.cpp b/wifi/1.0/default/wifi.cpp index 8feb8364ba..3d482b4e7e 100644 --- a/wifi/1.0/default/wifi.cpp +++ b/wifi/1.0/default/wifi.cpp @@ -85,8 +85,9 @@ Return Wifi::getChip(ChipId chip_id, getChip_cb hidl_status_cb) { WifiStatus Wifi::registerEventCallbackInternal( const sp& event_callback) { - // TODO(b/31632518): remove the callback when the client is destroyed - event_callbacks_.emplace_back(event_callback); + if (!event_cb_handler_.addCallback(event_callback)) { + return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); + } return createWifiStatus(WifiStatusCode::SUCCESS); } @@ -102,13 +103,13 @@ WifiStatus Wifi::startInternal() { // Create the chip instance once the HAL is started. chip_ = new WifiChip(kChipId, legacy_hal_, mode_controller_); run_state_ = RunState::STARTED; - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onStart().isOk()) { LOG(ERROR) << "Failed to invoke onStart callback"; }; } } else { - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onFailure(wifi_status).isOk()) { LOG(ERROR) << "Failed to invoke onFailure callback"; } @@ -126,13 +127,13 @@ WifiStatus Wifi::stopInternal() { } WifiStatus wifi_status = stopLegacyHalAndDeinitializeModeController(); if (wifi_status.code == WifiStatusCode::SUCCESS) { - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onStop().isOk()) { LOG(ERROR) << "Failed to invoke onStop callback"; }; } } else { - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onFailure(wifi_status).isOk()) { LOG(ERROR) << "Failed to invoke onFailure callback"; } diff --git a/wifi/1.0/default/wifi.h b/wifi/1.0/default/wifi.h index 40d3552568..c6fa84cb37 100644 --- a/wifi/1.0/default/wifi.h +++ b/wifi/1.0/default/wifi.h @@ -23,6 +23,7 @@ #include #include +#include "hidl_callback_util.h" #include "wifi_chip.h" #include "wifi_legacy_hal.h" #include "wifi_mode_controller.h" @@ -71,8 +72,8 @@ class Wifi : public IWifi { std::shared_ptr legacy_hal_; std::shared_ptr mode_controller_; RunState run_state_; - std::vector> event_callbacks_; sp chip_; + hidl_callback_util::HidlCallbackHandler event_cb_handler_; DISALLOW_COPY_AND_ASSIGN(Wifi); }; diff --git a/wifi/1.0/default/wifi_chip.cpp b/wifi/1.0/default/wifi_chip.cpp index 0e2d54e653..6aeedf827a 100644 --- a/wifi/1.0/default/wifi_chip.cpp +++ b/wifi/1.0/default/wifi_chip.cpp @@ -63,7 +63,7 @@ WifiChip::WifiChip( void WifiChip::invalidate() { invalidateAndRemoveAllIfaces(); legacy_hal_.reset(); - event_callbacks_.clear(); + event_cb_handler_.invalidate(); is_valid_ = false; } @@ -71,8 +71,8 @@ bool WifiChip::isValid() { return is_valid_; } -std::vector> WifiChip::getEventCallbacks() { - return event_callbacks_; +std::set> WifiChip::getEventCallbacks() { + return event_cb_handler_.getCallbacks(); } Return WifiChip::getId(getId_cb hidl_status_cb) { @@ -353,8 +353,9 @@ std::pair WifiChip::getIdInternal() { WifiStatus WifiChip::registerEventCallbackInternal( const sp& event_callback) { - // TODO(b/31632518): remove the callback when the client is destroyed - event_callbacks_.emplace_back(event_callback); + if (!event_cb_handler_.addCallback(event_callback)) { + return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); + } return createWifiStatus(WifiStatusCode::SUCCESS); } @@ -414,14 +415,14 @@ WifiStatus WifiChip::configureChipInternal(ChipModeId mode_id) { } WifiStatus status = handleChipConfiguration(mode_id); if (status.code != WifiStatusCode::SUCCESS) { - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onChipReconfigureFailure(status).isOk()) { LOG(ERROR) << "Failed to invoke onChipReconfigureFailure callback"; } } return status; } - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onChipReconfigured(mode_id).isOk()) { LOG(ERROR) << "Failed to invoke onChipReconfigured callback"; } @@ -503,7 +504,7 @@ std::pair> WifiChip::createApIfaceInternal() { } std::string ifname = legacy_hal_.lock()->getApIfaceName(); ap_iface_ = new WifiApIface(ifname, legacy_hal_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceAdded(IfaceType::AP, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceAdded callback"; } @@ -533,7 +534,7 @@ WifiStatus WifiChip::removeApIfaceInternal(const std::string& ifname) { return createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS); } invalidateAndClear(ap_iface_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceRemoved(IfaceType::AP, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceRemoved callback"; } @@ -549,7 +550,7 @@ std::pair> WifiChip::createNanIfaceInternal() { } std::string ifname = legacy_hal_.lock()->getNanIfaceName(); nan_iface_ = new WifiNanIface(ifname, legacy_hal_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceAdded(IfaceType::NAN, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceAdded callback"; } @@ -579,7 +580,7 @@ WifiStatus WifiChip::removeNanIfaceInternal(const std::string& ifname) { return createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS); } invalidateAndClear(nan_iface_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceRemoved(IfaceType::NAN, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceAdded callback"; } @@ -595,7 +596,7 @@ std::pair> WifiChip::createP2pIfaceInternal() { } std::string ifname = legacy_hal_.lock()->getP2pIfaceName(); p2p_iface_ = new WifiP2pIface(ifname, legacy_hal_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceAdded(IfaceType::P2P, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceAdded callback"; } @@ -625,7 +626,7 @@ WifiStatus WifiChip::removeP2pIfaceInternal(const std::string& ifname) { return createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS); } invalidateAndClear(p2p_iface_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceRemoved(IfaceType::P2P, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceRemoved callback"; } @@ -639,7 +640,7 @@ std::pair> WifiChip::createStaIfaceInternal() { } std::string ifname = legacy_hal_.lock()->getStaIfaceName(); sta_iface_ = new WifiStaIface(ifname, legacy_hal_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceAdded(IfaceType::STA, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceAdded callback"; } @@ -669,7 +670,7 @@ WifiStatus WifiChip::removeStaIfaceInternal(const std::string& ifname) { return createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS); } invalidateAndClear(sta_iface_); - for (const auto& callback : event_callbacks_) { + for (const auto& callback : event_cb_handler_.getCallbacks()) { if (!callback->onIfaceRemoved(IfaceType::STA, ifname).isOk()) { LOG(ERROR) << "Failed to invoke onIfaceRemoved callback"; } diff --git a/wifi/1.0/default/wifi_chip.h b/wifi/1.0/default/wifi_chip.h index 938b18077b..e1c234418b 100644 --- a/wifi/1.0/default/wifi_chip.h +++ b/wifi/1.0/default/wifi_chip.h @@ -22,6 +22,7 @@ #include #include +#include "hidl_callback_util.h" #include "wifi_ap_iface.h" #include "wifi_legacy_hal.h" #include "wifi_mode_controller.h" @@ -62,7 +63,7 @@ class WifiChip : public IWifiChip { // valid before processing them. void invalidate(); bool isValid(); - std::vector> getEventCallbacks(); + std::set> getEventCallbacks(); // HIDL methods exposed. Return getId(getId_cb hidl_status_cb) override; @@ -179,7 +180,6 @@ class WifiChip : public IWifiChip { ChipId chip_id_; std::weak_ptr legacy_hal_; std::weak_ptr mode_controller_; - std::vector> event_callbacks_; sp ap_iface_; sp nan_iface_; sp p2p_iface_; @@ -191,6 +191,8 @@ class WifiChip : public IWifiChip { // registration mechanism. Use this to check if we have already // registered a callback. bool debug_ring_buffer_cb_registered_; + hidl_callback_util::HidlCallbackHandler + event_cb_handler_; DISALLOW_COPY_AND_ASSIGN(WifiChip); }; diff --git a/wifi/1.0/default/wifi_nan_iface.cpp b/wifi/1.0/default/wifi_nan_iface.cpp index 8d76f91642..4165196494 100644 --- a/wifi/1.0/default/wifi_nan_iface.cpp +++ b/wifi/1.0/default/wifi_nan_iface.cpp @@ -55,7 +55,7 @@ WifiNanIface::WifiNanIface( switch (msg.response_type) { case legacy_hal::NAN_RESPONSE_ENABLED: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyEnableResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -63,7 +63,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_RESPONSE_DISABLED: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyDisableResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -71,7 +71,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_RESPONSE_PUBLISH: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyStartPublishResponse(id, wifiNanStatus, msg.body.publish_response.publish_id).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; @@ -80,7 +80,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_RESPONSE_PUBLISH_CANCEL: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyStopPublishResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -88,7 +88,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_RESPONSE_TRANSMIT_FOLLOWUP: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyTransmitFollowupResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -96,7 +96,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_RESPONSE_SUBSCRIBE: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyStartSubscribeResponse(id, wifiNanStatus, msg.body.subscribe_response.subscribe_id).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; @@ -105,7 +105,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_RESPONSE_SUBSCRIBE_CANCEL: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyStopSubscribeResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -113,7 +113,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_RESPONSE_CONFIG: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyConfigResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -127,7 +127,7 @@ WifiNanIface::WifiNanIface( LOG(ERROR) << "Failed to convert nan capabilities response"; return; } - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyCapabilitiesResponse(id, wifiNanStatus, hidl_struct).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; @@ -136,7 +136,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_DP_INTERFACE_CREATE: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyCreateDataInterfaceResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -144,7 +144,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_DP_INTERFACE_DELETE: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyDeleteDataInterfaceResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -152,7 +152,7 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_DP_INITIATOR_RESPONSE: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyInitiateDataPathResponse(id, wifiNanStatus, msg.body.data_request_response.ndp_instance_id).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; @@ -161,14 +161,14 @@ WifiNanIface::WifiNanIface( break; } case legacy_hal::NAN_DP_RESPONDER_RESPONSE: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyRespondToDataPathIndicationResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } } } case legacy_hal::NAN_DP_END: { - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->notifyTerminateDataPathResponse(id, wifiNanStatus).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -201,7 +201,7 @@ WifiNanIface::WifiNanIface( hidl_struct.eventType = (NanClusterEventType) msg.event_type; hidl_struct.addr = msg.data.mac_addr.addr; - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventClusterEvent(hidl_struct).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -219,7 +219,7 @@ WifiNanIface::WifiNanIface( status.status = hidl_struct_util::convertLegacyNanStatusTypeToHidl(msg.reason); status.description = msg.nan_reason; - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventDisabled(status).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -237,7 +237,7 @@ WifiNanIface::WifiNanIface( status.status = hidl_struct_util::convertLegacyNanStatusTypeToHidl(msg.reason); status.description = msg.nan_reason; - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventPublishTerminated(msg.publish_id, status).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -255,7 +255,7 @@ WifiNanIface::WifiNanIface( status.status = hidl_struct_util::convertLegacyNanStatusTypeToHidl(msg.reason); status.description = msg.nan_reason; - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventSubscribeTerminated(msg.subscribe_id, status).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -276,7 +276,7 @@ WifiNanIface::WifiNanIface( return; } - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventMatch(hidl_struct).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -290,7 +290,7 @@ WifiNanIface::WifiNanIface( LOG(ERROR) << "Callback invoked on an invalid object"; return; } - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventMatchExpired(msg.publish_subscribe_id, msg.requestor_instance_id).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; @@ -312,7 +312,7 @@ WifiNanIface::WifiNanIface( return; } - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventFollowupReceived(hidl_struct).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -330,7 +330,7 @@ WifiNanIface::WifiNanIface( status.status = hidl_struct_util::convertLegacyNanStatusTypeToHidl(msg.reason); status.description = msg.nan_reason; - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventTransmitFollowup(msg.id, status).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -351,7 +351,7 @@ WifiNanIface::WifiNanIface( return; } - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventDataPathRequest(hidl_struct).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -372,7 +372,7 @@ WifiNanIface::WifiNanIface( return; } - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { if (!callback->eventDataPathConfirm(hidl_struct).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; } @@ -386,7 +386,7 @@ WifiNanIface::WifiNanIface( LOG(ERROR) << "Callback invoked on an invalid object"; return; } - for (const auto& callback : shared_ptr_this->event_callbacks_) { + for (const auto& callback : shared_ptr_this->getEventCallbacks()) { for (int i = 0; i < msg.num_ndp_instances; ++i) { if (!callback->eventDataPathTerminated(msg.ndp_instance_id[i]).isOk()) { LOG(ERROR) << "Failed to invoke the callback"; @@ -410,7 +410,7 @@ WifiNanIface::WifiNanIface( void WifiNanIface::invalidate() { legacy_hal_.reset(); - event_callbacks_.clear(); + event_cb_handler_.invalidate(); is_valid_ = false; } @@ -418,6 +418,10 @@ bool WifiNanIface::isValid() { return is_valid_; } +std::set> WifiNanIface::getEventCallbacks() { + return event_cb_handler_.getCallbacks(); +} + Return WifiNanIface::getName(getName_cb hidl_status_cb) { return validateAndCall(this, WifiStatusCode::ERROR_WIFI_IFACE_INVALID, @@ -609,11 +613,9 @@ std::pair WifiNanIface::getTypeInternal() { WifiStatus WifiNanIface::registerEventCallbackInternal( const sp& callback) { - // TODO(b/31632518): remove the callback when the client is destroyed and/or - // make sure that the same callback is only registered once (i.e. detect duplicates) - // OR: consider having a single listener - not clear why multiple listeners (managers) are - // necessary, nor how they would coordinate (at least command IDs). - event_callbacks_.emplace_back(callback); + if (!event_cb_handler_.addCallback(callback)) { + return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); + } return createWifiStatus(WifiStatusCode::SUCCESS); } diff --git a/wifi/1.0/default/wifi_nan_iface.h b/wifi/1.0/default/wifi_nan_iface.h index d1da60efed..e1edd29243 100644 --- a/wifi/1.0/default/wifi_nan_iface.h +++ b/wifi/1.0/default/wifi_nan_iface.h @@ -21,6 +21,7 @@ #include #include +#include "hidl_callback_util.h" #include "wifi_legacy_hal.h" namespace android { @@ -119,10 +120,13 @@ class WifiNanIface : public IWifiNanIface { WifiStatus terminateDataPathRequestInternal( uint16_t cmd_id, uint32_t ndpInstanceId); + std::set> getEventCallbacks(); + std::string ifname_; std::weak_ptr legacy_hal_; - std::vector> event_callbacks_; bool is_valid_; + hidl_callback_util::HidlCallbackHandler + event_cb_handler_; DISALLOW_COPY_AND_ASSIGN(WifiNanIface); }; diff --git a/wifi/1.0/default/wifi_sta_iface.cpp b/wifi/1.0/default/wifi_sta_iface.cpp index 6100334f9c..55c9cf7062 100644 --- a/wifi/1.0/default/wifi_sta_iface.cpp +++ b/wifi/1.0/default/wifi_sta_iface.cpp @@ -35,7 +35,7 @@ WifiStaIface::WifiStaIface( void WifiStaIface::invalidate() { legacy_hal_.reset(); - event_callbacks_.clear(); + event_cb_handler_.invalidate(); is_valid_ = false; } @@ -43,8 +43,8 @@ bool WifiStaIface::isValid() { return is_valid_; } -std::vector> WifiStaIface::getEventCallbacks() { - return event_callbacks_; +std::set> WifiStaIface::getEventCallbacks() { + return event_cb_handler_.getCallbacks(); } Return WifiStaIface::getName(getName_cb hidl_status_cb) { @@ -293,8 +293,9 @@ std::pair WifiStaIface::getTypeInternal() { WifiStatus WifiStaIface::registerEventCallbackInternal( const sp& callback) { - // TODO(b/31632518): remove the callback when the client is destroyed - event_callbacks_.emplace_back(callback); + if (!event_cb_handler_.addCallback(callback)) { + return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN); + } return createWifiStatus(WifiStatusCode::SUCCESS); } @@ -570,7 +571,8 @@ WifiStatus WifiStaIface::stopSendingKeepAlivePacketsInternal(uint32_t cmd_id) { return createWifiStatusFromLegacyError(legacy_status); } -WifiStatus WifiStaIface::setScanningMacOuiInternal(const std::array& oui) { +WifiStatus WifiStaIface::setScanningMacOuiInternal( + const std::array& oui) { legacy_hal::wifi_error legacy_status = legacy_hal_.lock()->setScanningMacOui(oui); return createWifiStatusFromLegacyError(legacy_status); diff --git a/wifi/1.0/default/wifi_sta_iface.h b/wifi/1.0/default/wifi_sta_iface.h index bc2d75fea8..5f0ffe9a2c 100644 --- a/wifi/1.0/default/wifi_sta_iface.h +++ b/wifi/1.0/default/wifi_sta_iface.h @@ -21,6 +21,7 @@ #include #include +#include "hidl_callback_util.h" #include "wifi_legacy_hal.h" namespace android { @@ -39,7 +40,7 @@ class WifiStaIface : public IWifiStaIface { // Refer to |WifiChip::invalidate()|. void invalidate(); bool isValid(); - std::vector> getEventCallbacks(); + std::set> getEventCallbacks(); // HIDL methods exposed. Return getName(getName_cb hidl_status_cb) override; @@ -151,8 +152,9 @@ class WifiStaIface : public IWifiStaIface { std::string ifname_; std::weak_ptr legacy_hal_; - std::vector> event_callbacks_; bool is_valid_; + hidl_callback_util::HidlCallbackHandler + event_cb_handler_; DISALLOW_COPY_AND_ASSIGN(WifiStaIface); };