Set HAL response functions after framework sets all of theirs.

Bug: 207695009
Test: Build with ag/16322062 and a handful of patches from b/207695009,
      try to use telephony stack
Change-Id: I74afe105a22a24efa0c38de20c75beffc58b144f
Merged-In: I74afe105a22a24efa0c38de20c75beffc58b144f

radio/aidl/compat/libradiocompat/Android.bp

@@ -55,6 +55,7 @@ cc_library {
         "libutils",
     ],
     srcs: [
+        "CallbackManager.cpp",
         "DriverContext.cpp",
         "RadioCompatBase.cpp",
         "RadioIndication.cpp",

radio/aidl/compat/libradiocompat/CallbackManager.cpp

@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2021 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 <libradiocompat/CallbackManager.h>
+
+#include <android-base/logging.h>
+
+using namespace std::literals::chrono_literals;
+
+namespace android::hardware::radio::compat {
+
+/**
+ * How much setter thread will wait with setting response functions after the last
+ * setResponseFunctions call from the framework. Subsequent calls from the framework reset the
+ * clock, so this number should be larger than the longest time between setResponseFunctions calls
+ * from the framework.
+ *
+ * Real world measurements with Cuttlefish give <10ms delay between Modem and Data and <2ms delays
+ * between all others.
+ */
+static constexpr auto kDelayedSetterDelay = 100ms;
+
+CallbackManager::CallbackManager(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal)
+    : mHidlHal(hidlHal),
+      mRadioResponse(sp<compat::RadioResponse>::make(context)),
+      mRadioIndication(sp<compat::RadioIndication>::make(context)),
+      mDelayedSetterThread(&CallbackManager::delayedSetterThread, this) {}
+
+CallbackManager::~CallbackManager() {
+    {
+        std::unique_lock<std::mutex> lock(mDelayedSetterGuard);
+        mDelayedSetterDeadline = std::nullopt;
+        mDestroy = true;
+        mDelayedSetterCv.notify_all();
+    }
+    mDelayedSetterThread.join();
+}
+
+RadioResponse& CallbackManager::response() const {
+    return *mRadioResponse;
+}
+
+void CallbackManager::setResponseFunctionsDelayed() {
+    std::unique_lock<std::mutex> lock(mDelayedSetterGuard);
+    mDelayedSetterDeadline = std::chrono::steady_clock::now() + kDelayedSetterDelay;
+    mDelayedSetterCv.notify_all();
+}
+
+void CallbackManager::delayedSetterThread() {
+    while (!mDestroy) {
+        std::unique_lock<std::mutex> lock(mDelayedSetterGuard);
+        auto deadline = mDelayedSetterDeadline;
+
+        // not waiting to set response functions
+        if (!deadline) {
+            mDelayedSetterCv.wait(lock);
+            continue;
+        }
+
+        // waiting to set response functions, but not yet
+        if (*deadline > std::chrono::steady_clock::now()) {
+            mDelayedSetterCv.wait_until(lock, *deadline);
+            continue;
+        }
+
+        mHidlHal->setResponseFunctions(mRadioResponse, mRadioIndication).assertOk();
+        mDelayedSetterDeadline = std::nullopt;
+    }
+}
+
+}  // namespace android::hardware::radio::compat

radio/aidl/compat/libradiocompat/RadioCompatBase.cpp

@@ -21,11 +21,10 @@
 namespace android::hardware::radio::compat {
 
 RadioCompatBase::RadioCompatBase(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal,
-                                 sp<RadioResponse> radioResponse, sp<RadioIndication> radioInd)
+                                 std::shared_ptr<CallbackManager> cbMgr)
     : mContext(context),
       mHal1_5(hidlHal),
       mHal1_6(V1_6::IRadio::castFrom(hidlHal)),
-      mRadioResponse(radioResponse),
-      mRadioIndication(radioInd) {}
+      mCallbackManager(cbMgr) {}
 
 }  // namespace android::hardware::radio::compat

radio/aidl/compat/libradiocompat/config/RadioConfig.cpp

@@ -88,7 +88,7 @@ ScopedAStatus RadioConfig::setResponseFunctions(
 
     mRadioConfigResponse->setResponseFunction(radioConfigResponse);
     mRadioConfigIndication->setResponseFunction(radioConfigIndication);
-    mHal1_1->setResponseFunctions(mRadioConfigResponse, mRadioConfigIndication);
+    mHal1_1->setResponseFunctions(mRadioConfigResponse, mRadioConfigIndication).assertOk();
 
     return ok();
 }

radio/aidl/compat/libradiocompat/data/RadioData.cpp

@@ -32,7 +32,7 @@ namespace aidlCommon = ::aidl::android::hardware::radio;
 constexpr auto ok = &ScopedAStatus::ok;
 
 std::shared_ptr<aidl::IRadioDataResponse> RadioData::respond() {
-    return mRadioResponse->dataCb();
+    return mCallbackManager->response().dataCb();
 }
 
 ScopedAStatus RadioData::allocatePduSessionId(int32_t serial) {
@@ -129,16 +129,10 @@ ScopedAStatus RadioData::setInitialAttachApn(int32_t serial, const aidl::DataPro
 }
 
 ScopedAStatus RadioData::setResponseFunctions(
-        const std::shared_ptr<aidl::IRadioDataResponse>& dataResponse,
-        const std::shared_ptr<aidl::IRadioDataIndication>& dataIndication) {
-    LOG_CALL << dataResponse << ' ' << dataIndication;
-
-    CHECK(dataResponse);
-    CHECK(dataIndication);
-
-    mRadioResponse->setResponseFunction(dataResponse);
-    mRadioIndication->setResponseFunction(dataIndication);
-
+        const std::shared_ptr<aidl::IRadioDataResponse>& response,
+        const std::shared_ptr<aidl::IRadioDataIndication>& indication) {
+    LOG_CALL << response << ' ' << indication;
+    mCallbackManager->setResponseFunctions(response, indication);
     return ok();
 }
 

radio/aidl/compat/libradiocompat/include/libradiocompat/CallbackManager.h

@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+#pragma once
+
+#include "DriverContext.h"
+#include "RadioIndication.h"
+#include "RadioResponse.h"
+
+#include <android-base/logging.h>
+#include <android/hardware/radio/1.6/IRadio.h>
+
+#include <thread>
+
+namespace android::hardware::radio::compat {
+
+class CallbackManager {
+    sp<V1_5::IRadio> mHidlHal;
+    sp<RadioResponse> mRadioResponse;
+    sp<RadioIndication> mRadioIndication;
+
+    std::thread mDelayedSetterThread;
+    std::mutex mDelayedSetterGuard;
+    std::optional<std::chrono::time_point<std::chrono::steady_clock>> mDelayedSetterDeadline
+            GUARDED_BY(mDelayedSetterGuard);
+    std::condition_variable mDelayedSetterCv GUARDED_BY(mDelayedSetterGuard);
+    bool mDestroy GUARDED_BY(mDelayedSetterGuard) = false;
+
+    void setResponseFunctionsDelayed();
+    void delayedSetterThread();
+
+  public:
+    CallbackManager(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal);
+    ~CallbackManager();
+
+    RadioResponse& response() const;
+
+    template <typename ResponseType, typename IndicationType>
+    void setResponseFunctions(const std::shared_ptr<ResponseType>& response,
+                              const std::shared_ptr<IndicationType>& indication) {
+        CHECK(response);
+        CHECK(indication);
+
+        mRadioResponse->setResponseFunction(response);
+        mRadioIndication->setResponseFunction(indication);
+        setResponseFunctionsDelayed();
+    }
+};
+
+}  // namespace android::hardware::radio::compat

radio/aidl/compat/libradiocompat/include/libradiocompat/RadioCompatBase.h

@@ -15,9 +15,8 @@
  */
 #pragma once
 
+#include "CallbackManager.h"
 #include "DriverContext.h"
-#include "RadioIndication.h"
-#include "RadioResponse.h"
 
 #include <android/hardware/radio/1.6/IRadio.h>
 
@@ -30,12 +29,11 @@ class RadioCompatBase {
     sp<V1_5::IRadio> mHal1_5;
     sp<V1_6::IRadio> mHal1_6;
 
-    sp<RadioResponse> mRadioResponse;
-    sp<RadioIndication> mRadioIndication;
+    std::shared_ptr<CallbackManager> mCallbackManager;
 
   public:
     RadioCompatBase(std::shared_ptr<DriverContext> context, sp<V1_5::IRadio> hidlHal,
-                    sp<RadioResponse> radioResponse, sp<RadioIndication> radioIndication);
+                    std::shared_ptr<CallbackManager> cbMgr);
 };
 
 }  // namespace android::hardware::radio::compat

radio/aidl/compat/libradiocompat/messaging/RadioMessaging.cpp

@@ -30,7 +30,7 @@ namespace aidl = ::aidl::android::hardware::radio::messaging;
 constexpr auto ok = &ScopedAStatus::ok;
 
 std::shared_ptr<aidl::IRadioMessagingResponse> RadioMessaging::respond() {
-    return mRadioResponse->messagingCb();
+    return mCallbackManager->response().messagingCb();
 }
 
 ScopedAStatus RadioMessaging::acknowledgeIncomingGsmSmsWithPdu(  //
@@ -181,16 +181,10 @@ ScopedAStatus RadioMessaging::setGsmBroadcastConfig(
 }
 
 ScopedAStatus RadioMessaging::setResponseFunctions(
-        const std::shared_ptr<aidl::IRadioMessagingResponse>& messagingResponse,
-        const std::shared_ptr<aidl::IRadioMessagingIndication>& messagingIndication) {
-    LOG_CALL << messagingResponse << ' ' << messagingIndication;
-
-    CHECK(messagingResponse);
-    CHECK(messagingIndication);
-
-    mRadioResponse->setResponseFunction(messagingResponse);
-    mRadioIndication->setResponseFunction(messagingIndication);
-
+        const std::shared_ptr<aidl::IRadioMessagingResponse>& response,
+        const std::shared_ptr<aidl::IRadioMessagingIndication>& indication) {
+    LOG_CALL << response << ' ' << indication;
+    mCallbackManager->setResponseFunctions(response, indication);
     return ok();
 }
 

radio/aidl/compat/libradiocompat/modem/RadioModem.cpp

@@ -28,7 +28,7 @@ namespace aidl = ::aidl::android::hardware::radio::modem;
 constexpr auto ok = &ScopedAStatus::ok;
 
 std::shared_ptr<aidl::IRadioModemResponse> RadioModem::respond() {
-    return mRadioResponse->modemCb();
+    return mCallbackManager->response().modemCb();
 }
 
 ScopedAStatus RadioModem::enableModem(int32_t serial, bool on) {
@@ -133,16 +133,10 @@ ScopedAStatus RadioModem::setRadioPower(int32_t serial, bool powerOn, bool forEm
 }
 
 ScopedAStatus RadioModem::setResponseFunctions(
-        const std::shared_ptr<aidl::IRadioModemResponse>& modemResponse,
-        const std::shared_ptr<aidl::IRadioModemIndication>& modemIndication) {
-    LOG_CALL << modemResponse << ' ' << modemIndication;
-
-    CHECK(modemResponse);
-    CHECK(modemIndication);
-
-    mRadioResponse->setResponseFunction(modemResponse);
-    mRadioIndication->setResponseFunction(modemIndication);
-
+        const std::shared_ptr<aidl::IRadioModemResponse>& response,
+        const std::shared_ptr<aidl::IRadioModemIndication>& indication) {
+    LOG_CALL << response << ' ' << indication;
+    mCallbackManager->setResponseFunctions(response, indication);
     return ok();
 }
 

radio/aidl/compat/libradiocompat/network/RadioNetwork.cpp

@@ -34,7 +34,7 @@ namespace aidl = ::aidl::android::hardware::radio::network;
 constexpr auto ok = &ScopedAStatus::ok;
 
 std::shared_ptr<aidl::IRadioNetworkResponse> RadioNetwork::respond() {
-    return mRadioResponse->networkCb();
+    return mCallbackManager->response().networkCb();
 }
 
 ScopedAStatus RadioNetwork::getAllowedNetworkTypesBitmap(int32_t serial) {
@@ -245,16 +245,10 @@ ScopedAStatus RadioNetwork::setNrDualConnectivityState(int32_t serial,
 }
 
 ScopedAStatus RadioNetwork::setResponseFunctions(
-        const std::shared_ptr<aidl::IRadioNetworkResponse>& networkResponse,
-        const std::shared_ptr<aidl::IRadioNetworkIndication>& networkIndication) {
-    LOG_CALL << networkResponse << ' ' << networkIndication;
-
-    CHECK(networkResponse);
-    CHECK(networkIndication);
-
-    mRadioResponse->setResponseFunction(networkResponse);
-    mRadioIndication->setResponseFunction(networkIndication);
-
+        const std::shared_ptr<aidl::IRadioNetworkResponse>& response,
+        const std::shared_ptr<aidl::IRadioNetworkIndication>& indication) {
+    LOG_CALL << response << ' ' << indication;
+    mCallbackManager->setResponseFunctions(response, indication);
     return ok();
 }
 

radio/aidl/compat/libradiocompat/sim/RadioSim.cpp

@@ -31,7 +31,7 @@ namespace aidl = ::aidl::android::hardware::radio::sim;
 constexpr auto ok = &ScopedAStatus::ok;
 
 std::shared_ptr<aidl::IRadioSimResponse> RadioSim::respond() {
-    return mRadioResponse->simCb();
+    return mCallbackManager->response().simCb();
 }
 
 ScopedAStatus RadioSim::areUiccApplicationsEnabled(int32_t serial) {
@@ -221,16 +221,10 @@ ScopedAStatus RadioSim::setFacilityLockForApp(  //
 }
 
 ScopedAStatus RadioSim::setResponseFunctions(
-        const std::shared_ptr<aidl::IRadioSimResponse>& simResponse,
-        const std::shared_ptr<aidl::IRadioSimIndication>& simIndication) {
-    LOG_CALL << simResponse << ' ' << simIndication;
-
-    CHECK(simResponse);
-    CHECK(simIndication);
-
-    mRadioResponse->setResponseFunction(simResponse);
-    mRadioIndication->setResponseFunction(simIndication);
-
+        const std::shared_ptr<aidl::IRadioSimResponse>& response,
+        const std::shared_ptr<aidl::IRadioSimIndication>& indication) {
+    LOG_CALL << response << ' ' << indication;
+    mCallbackManager->setResponseFunctions(response, indication);
     return ok();
 }
 

radio/aidl/compat/libradiocompat/voice/RadioVoice.cpp

@@ -31,7 +31,7 @@ namespace aidl = ::aidl::android::hardware::radio::voice;
 constexpr auto ok = &ScopedAStatus::ok;
 
 std::shared_ptr<aidl::IRadioVoiceResponse> RadioVoice::respond() {
-    return mRadioResponse->voiceCb();
+    return mCallbackManager->response().voiceCb();
 }
 
 ScopedAStatus RadioVoice::acceptCall(int32_t serial) {
@@ -238,16 +238,10 @@ ScopedAStatus RadioVoice::setPreferredVoicePrivacy(int32_t serial, bool enable)
 }
 
 ScopedAStatus RadioVoice::setResponseFunctions(
-        const std::shared_ptr<aidl::IRadioVoiceResponse>& voiceResponse,
-        const std::shared_ptr<aidl::IRadioVoiceIndication>& voiceIndication) {
-    LOG_CALL << voiceResponse << ' ' << voiceIndication;
-
-    CHECK(voiceResponse);
-    CHECK(voiceIndication);
-
-    mRadioResponse->setResponseFunction(voiceResponse);
-    mRadioIndication->setResponseFunction(voiceIndication);
-
+        const std::shared_ptr<aidl::IRadioVoiceResponse>& response,
+        const std::shared_ptr<aidl::IRadioVoiceIndication>& indication) {
+    LOG_CALL << response << ' ' << indication;
+    mCallbackManager->setResponseFunctions(response, indication);
     return ok();
 }
 

radio/aidl/compat/service/service.cpp

@@ -19,13 +19,12 @@
 #include <android-base/logging.h>
 #include <android/binder_manager.h>
 #include <android/binder_process.h>
+#include <libradiocompat/CallbackManager.h>
 #include <libradiocompat/RadioConfig.h>
 #include <libradiocompat/RadioData.h>
-#include <libradiocompat/RadioIndication.h>
 #include <libradiocompat/RadioMessaging.h>
 #include <libradiocompat/RadioModem.h>
 #include <libradiocompat/RadioNetwork.h>
-#include <libradiocompat/RadioResponse.h>
 #include <libradiocompat/RadioSim.h>
 #include <libradiocompat/RadioVoice.h>
 
@@ -36,9 +35,8 @@ using namespace std::string_literals;
 static std::vector<std::shared_ptr<ndk::ICInterface>> gPublishedHals;
 
 template <typename T>
-static void publishRadioHal(std::shared_ptr<compat::DriverContext> context,
-                            sp<V1_5::IRadio> hidlHal, sp<compat::RadioResponse> responseCb,
-                            sp<compat::RadioIndication> indicationCb, const std::string& slot) {
+static void publishRadioHal(std::shared_ptr<compat::DriverContext> ctx, sp<V1_5::IRadio> hidlHal,
+                            std::shared_ptr<compat::CallbackManager> cm, const std::string& slot) {
     const auto instance = T::descriptor + "/"s + slot;
     if (!AServiceManager_isDeclared(instance.c_str())) {
         LOG(INFO) << instance << " is not declared in VINTF (this may be intentional)";
@@ -46,7 +44,7 @@ static void publishRadioHal(std::shared_ptr<compat::DriverContext> context,
     }
     LOG(DEBUG) << "Publishing " << instance;
 
-    auto aidlHal = ndk::SharedRefBase::make<T>(context, hidlHal, responseCb, indicationCb);
+    auto aidlHal = ndk::SharedRefBase::make<T>(ctx, hidlHal, cm);
     gPublishedHals.push_back(aidlHal);
     const auto status = AServiceManager_addService(aidlHal->asBinder().get(), instance.c_str());
     CHECK_EQ(status, STATUS_OK);
@@ -59,17 +57,14 @@ static void publishRadio(std::string slot) {
     hidl_utils::linkDeathToDeath(radioHidl);
 
     auto context = std::make_shared<compat::DriverContext>();
+    auto callbackMgr = std::make_shared<compat::CallbackManager>(context, radioHidl);
 
-    auto responseCb = sp<compat::RadioResponse>::make(context);
-    auto indicationCb = sp<compat::RadioIndication>::make(context);
-    radioHidl->setResponseFunctions(responseCb, indicationCb).assertOk();
-
-    publishRadioHal<compat::RadioData>(context, radioHidl, responseCb, indicationCb, slot);
-    publishRadioHal<compat::RadioMessaging>(context, radioHidl, responseCb, indicationCb, slot);
-    publishRadioHal<compat::RadioModem>(context, radioHidl, responseCb, indicationCb, slot);
-    publishRadioHal<compat::RadioNetwork>(context, radioHidl, responseCb, indicationCb, slot);
-    publishRadioHal<compat::RadioSim>(context, radioHidl, responseCb, indicationCb, slot);
-    publishRadioHal<compat::RadioVoice>(context, radioHidl, responseCb, indicationCb, slot);
+    publishRadioHal<compat::RadioData>(context, radioHidl, callbackMgr, slot);
+    publishRadioHal<compat::RadioMessaging>(context, radioHidl, callbackMgr, slot);
+    publishRadioHal<compat::RadioModem>(context, radioHidl, callbackMgr, slot);
+    publishRadioHal<compat::RadioNetwork>(context, radioHidl, callbackMgr, slot);
+    publishRadioHal<compat::RadioSim>(context, radioHidl, callbackMgr, slot);
+    publishRadioHal<compat::RadioVoice>(context, radioHidl, callbackMgr, slot);
 }
 
 static void publishRadioConfig() {