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

Commit fbfa1ced authored by Hungming Chen's avatar Hungming Chen
Browse files

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

The NDK binder doesn't pass dead binder pointer to binder death handler.
The binder death handler can't know who was died as IBinder binderDied()
does. An alternative way is to use the raw address of binder interface
object to be an identification of dead binder.

Test: built, flashed, booted
      system/netd/tests/runtests.sh passes

Change-Id: I3c6f2840ddc51c85f417c1b0b4b0717e297f8a71
parent c6bbbe71
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();
    {