Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6b5603f8 authored by Hungming Chen's avatar Hungming Chen Committed by android-build-merger
Browse files

resolv: Use the raw binder pointer address to identify dead binder

am: ef137d1964

Change-Id: I1c4a7b85280de023950e6f5538bfaa83b47cee72
parents 6edd4581 fbfa1ced
Loading
Loading
Loading
Loading
+31 −32
Original line number Diff line number Diff line
@@ -15,23 +15,20 @@
 */
#define LOG_TAG "ResolverEventReporter"

#include "ResolverEventReporter.h"

#include <android-base/logging.h>
#include <android/binder_manager.h>

#include "ResolverEventReporter.h"

using aidl::android::net::metrics::INetdEventListener;

ResolverEventReporter& ResolverEventReporter::getInstance() {
    // It should be initialized only once.
    static ResolverEventReporter instance;

    // Add default listener which comes from framework. The framework listener "netd_listener"
    // should be added only once if it has been added successfully.
    // TODO: Consider registering default listener from framework.
    // Currently, the framework listener "netd_listener" is shared by netd and libnetd_resolv.
    // Consider breaking it into two listeners. Once it has done, may let framework register
    // the listener proactively.
    // Add framework metrics listener. Because the binder service "netd_listener" may be launched
    // later than Netd, try to get binder handler in every instance query if any. The framework
    // metrics listener should be added only once if it has been already added successfully.
    instance.addDefaultListener();

    return instance;
@@ -45,6 +42,10 @@ int ResolverEventReporter::addListener(const std::shared_ptr<INetdEventListener>
    return addListenerImpl(listener);
}

// TODO: Consider registering metrics listener from framework and remove this function.
// Currently, the framework listener "netd_listener" is shared by netd and libnetd_resolv.
// Consider breaking it into two listeners. Once it has done, may let framework register
// the listener proactively.
void ResolverEventReporter::addDefaultListener() {
    std::lock_guard lock(mMutex);

@@ -61,25 +62,15 @@ void ResolverEventReporter::addDefaultListener() {
    if (!addListenerImplLocked(listener)) added = true;
}

void ResolverEventReporter::handleBinderDied() {
void ResolverEventReporter::handleBinderDied(const void* who) {
    std::lock_guard lock(mMutex);

    for (const auto& it : mListeners) {
        // TODO: Considering that find a way to identify dead binder if binder ndk has supported.
        // b/128712772.
        // Currently, binder ndk doesn't pass dead binder pointer to death handle function as
        // IBinder.DeathRecipient binderDied() does. The death handle function doesn't directly
        // know which binder was dead. This is a workaround which just removes the first found dead
        // binder in map. It doesn't guarantee that the first found dead binder is the real death
        // trigger. It should be okay sa far because this death handle function is only used for
        // the listener which registers from unit test and there has only one listener unit test
        // case now. In normal case, Netd should be killed if framework is dead. Don't need to
        // handle the death of framework listener. For long term, this should be fixed.
        if (!AIBinder_isAlive(it->asBinder().get())) {
            mListeners.erase(it);
            return;
        }
    }
    // Use the raw binder pointer address to be the identification of dead binder. Treat "who"
    // which passes the raw address of dead binder as an identification only.
    auto found = std::find_if(mListeners.begin(), mListeners.end(),
                              [=](const auto& it) { return static_cast<void*>(it.get()) == who; });

    if (found != mListeners.end()) mListeners.erase(found);
}

ResolverEventReporter::ListenerSet ResolverEventReporter::getListenersImpl() const {
@@ -99,27 +90,35 @@ int ResolverEventReporter::addListenerImplLocked(
        return -EINVAL;
    }

    // TODO: Perhaps ignore the listener which has the same binder.
    // TODO: Perhaps ignore the listener which comes from the same binder.
    const auto& it = mListeners.find(listener);
    if (it != mListeners.end()) {
        LOG(WARNING) << "The listener was already subscribed";
        return -EEXIST;
    }

    if (mDeathRecipient == nullptr) {
    static AIBinder_DeathRecipient* deathRecipient = nullptr;
    if (deathRecipient == nullptr) {
        // The AIBinder_DeathRecipient object is used to manage all death recipients for multiple
        // binder objects. It doesn't released because there should have at least one binder object
        // from framework.
        // TODO: Considering to remove death recipient for the binder object from framework because
        // it doesn't need death recipient actually.
        mDeathRecipient = AIBinder_DeathRecipient_new([](void* cookie) {
            auto onDeath = static_cast<ResolverEventReporter::OnDeathFunc*>(cookie);
            (*onDeath)();
        deathRecipient = AIBinder_DeathRecipient_new([](void* cookie) {
            ResolverEventReporter::getInstance().handleBinderDied(cookie);
        });
    }

    binder_status_t status = AIBinder_linkToDeath(listener->asBinder().get(), mDeathRecipient,
                                                  static_cast<void*>(&mOnDeath));
    // Pass the raw binder pointer address to be the cookie of the death recipient. While the death
    // notification is fired, the cookie is used for identifying which binder was died. Because
    // the NDK binder doesn't pass dead binder pointer to binder death handler, the binder death
    // handler can't know who was died via wp<IBinder>. The reason for wp<IBinder> is not passed
    // is that NDK binder can't transform a wp<IBinder> to a wp<AIBinder> in some cases.
    // See more information in b/128712772.
    auto binder = listener->asBinder().get();
    auto cookie = static_cast<void*>(listener.get());  // Used for dead binder identification.
    binder_status_t status = AIBinder_linkToDeath(binder, deathRecipient, cookie);

    if (STATUS_OK != status) {
        LOG(ERROR) << "Failed to register death notification for INetdEventListener";
        return -EAGAIN;
+2 −5
Original line number Diff line number Diff line
@@ -36,7 +36,6 @@ class ResolverEventReporter {
    ResolverEventReporter& operator=(ResolverEventReporter&&) = delete;

    using ListenerSet = std::set<std::shared_ptr<aidl::android::net::metrics::INetdEventListener>>;
    using OnDeathFunc = std::function<void(void)>;

    // Get the instance of the singleton ResolverEventReporter.
    static ResolverEventReporter& getInstance();
@@ -49,7 +48,7 @@ class ResolverEventReporter {
            const std::shared_ptr<aidl::android::net::metrics::INetdEventListener>& listener);

  private:
    ResolverEventReporter() : mDeathRecipient(nullptr){};
    ResolverEventReporter() = default;
    ~ResolverEventReporter() = default;

    void addDefaultListener() EXCLUDES(mMutex);
@@ -60,12 +59,10 @@ class ResolverEventReporter {
            const std::shared_ptr<aidl::android::net::metrics::INetdEventListener>& listener)
            REQUIRES(mMutex);
    ListenerSet getListenersImpl() const EXCLUDES(mMutex);
    void handleBinderDied() EXCLUDES(mMutex);
    void handleBinderDied(const void* who) EXCLUDES(mMutex);

    mutable std::mutex mMutex;
    ListenerSet mListeners GUARDED_BY(mMutex);
    AIBinder_DeathRecipient* mDeathRecipient GUARDED_BY(mMutex);
    OnDeathFunc mOnDeath = [&] { handleBinderDied(); };
};

#endif  // NETD_RESOLV_EVENT_REPORTER_H
+3 −3
Original line number Diff line number Diff line
@@ -143,14 +143,14 @@ TEST_F(DnsResolverBinderTest, EventListener_onDnsEvent) {
    dns.clearQueries();

    // Register event listener.
    TestOnDnsEvent* testOnDnsEvent = new TestOnDnsEvent(expectedResults);
    android::sp<TestOnDnsEvent> testOnDnsEvent = new TestOnDnsEvent(expectedResults);
    android::binder::Status status = mDnsResolver->registerEventListener(
            android::interface_cast<INetdEventListener>(testOnDnsEvent));
    ASSERT_TRUE(status.isOk()) << status.exceptionMessage();

    // DNS queries.
    // Once all expected events of expectedResults are received by the listener. The unit test will
    // be notified and the verified flag Event::onDnsEvent of class TestOnDnsEvent will be set.
    // Once all expected events of expectedResults are received by the listener, the unit test will
    // be notified. Otherwise, notified with a timeout expired failure.
    auto& cv = testOnDnsEvent->getCv();
    auto& cvMutex = testOnDnsEvent->getCvMutex();
    {