Fix DeathRecipient not work problem for UnsolicitedEventListener
linkToDeath of binder only use weak pointer ref for deathRecipient.
Hence local variable deathRecipient would destruct immediately after
registerUnsolEventListener function done.
Use a map to store all deathRecipient.
Bug: 129370555
Test: built, flashed, booted
system/netd/tests/runtests.sh pass
Change-Id: I1829222dc071e237974033559bfbcce72b05d3fb
diff --git a/server/EventReporter.cpp b/server/EventReporter.cpp
index 52dd5c9..64a1393 100644
--- a/server/EventReporter.cpp
+++ b/server/EventReporter.cpp
@@ -38,35 +38,41 @@
return mNetdEventListener;
}
-EventReporter::UnsolListeners EventReporter::getNetdUnsolicitedEventListeners() {
+EventReporter::UnsolListenerMap EventReporter::getNetdUnsolicitedEventListenerMap() const {
std::lock_guard lock(mUnsolicitedMutex);
- return mUnsolListeners;
+ return mUnsolListenerMap;
}
void EventReporter::registerUnsolEventListener(
const android::sp<INetdUnsolicitedEventListener>& listener) {
std::lock_guard lock(mUnsolicitedMutex);
- mUnsolListeners.insert(listener);
// Create the death listener.
class DeathRecipient : public android::IBinder::DeathRecipient {
public:
- DeathRecipient(UnsolListeners* listeners,
- android::sp<INetdUnsolicitedEventListener> listener, std::mutex& unsolMutex)
- : mMutex(unsolMutex), mUnsolListeners(listeners), mListener(std::move(listener)) {}
+ DeathRecipient(EventReporter* eventReporter,
+ android::sp<INetdUnsolicitedEventListener> listener)
+ : mEventReporter(eventReporter), mListener(std::move(listener)) {}
~DeathRecipient() override = default;
-
- private:
void binderDied(const android::wp<android::IBinder>& /* who */) override {
- std::lock_guard lock(mMutex);
- mUnsolListeners->erase(mListener);
+ mEventReporter->unregisterUnsolEventListener(mListener);
}
- std::mutex& mMutex;
- UnsolListeners* mUnsolListeners GUARDED_BY(mMutex);
+ private:
+ EventReporter* mEventReporter;
android::sp<INetdUnsolicitedEventListener> mListener;
};
android::sp<android::IBinder::DeathRecipient> deathRecipient =
- new DeathRecipient(&mUnsolListeners, listener, mUnsolicitedMutex);
+ new DeathRecipient(this, listener);
+
android::IInterface::asBinder(listener)->linkToDeath(deathRecipient);
+
+ // TODO: Consider to use remote binder address as registering key
+ mUnsolListenerMap.insert({listener, deathRecipient});
+}
+
+void EventReporter::unregisterUnsolEventListener(
+ const android::sp<INetdUnsolicitedEventListener>& listener) {
+ std::lock_guard lock(mUnsolicitedMutex);
+ mUnsolListenerMap.erase(listener);
}
diff --git a/server/EventReporter.h b/server/EventReporter.h
index f471d7f..934aa7a 100644
--- a/server/EventReporter.h
+++ b/server/EventReporter.h
@@ -17,8 +17,8 @@
#ifndef NETD_SERVER_EVENT_REPORTER_H
#define NETD_SERVER_EVENT_REPORTER_H
+#include <map>
#include <mutex>
-#include <set>
#include <android-base/thread_annotations.h>
#include <binder/IServiceManager.h>
@@ -31,25 +31,30 @@
*/
class EventReporter {
public:
- using UnsolListeners = std::set<const android::sp<android::net::INetdUnsolicitedEventListener>>;
+ using UnsolListenerMap =
+ std::map<const android::sp<android::net::INetdUnsolicitedEventListener>,
+ const android::sp<android::IBinder::DeathRecipient>>;
// Returns the binder reference to the netd events listener service, attempting to fetch it if
// we do not have it already. This method is threadsafe.
android::sp<android::net::metrics::INetdEventListener> getNetdEventListener();
// Returns a copy of the registered listeners.
- UnsolListeners getNetdUnsolicitedEventListeners() EXCLUDES(mUnsolicitedMutex);
+ UnsolListenerMap getNetdUnsolicitedEventListenerMap() const EXCLUDES(mUnsolicitedMutex);
void registerUnsolEventListener(
const android::sp<android::net::INetdUnsolicitedEventListener>& listener)
EXCLUDES(mUnsolicitedMutex);
+ void unregisterUnsolEventListener(
+ const android::sp<android::net::INetdUnsolicitedEventListener>& listener)
+ EXCLUDES(mUnsolicitedMutex);
private:
std::mutex mEventMutex;
- std::mutex mUnsolicitedMutex;
+ mutable std::mutex mUnsolicitedMutex;
android::sp<android::net::metrics::INetdEventListener> mNetdEventListener
GUARDED_BY(mEventMutex);
- UnsolListeners mUnsolListeners GUARDED_BY(mUnsolicitedMutex);
+ UnsolListenerMap mUnsolListenerMap GUARDED_BY(mUnsolicitedMutex);
};
#endif // NETD_SERVER_EVENT_REPORTER_H
diff --git a/server/NetlinkHandler.cpp b/server/NetlinkHandler.cpp
index 0354fca..7fb3437 100644
--- a/server/NetlinkHandler.cpp
+++ b/server/NetlinkHandler.cpp
@@ -51,15 +51,15 @@
res; \
})
-#define LOG_EVENT_FUNC(retry, func, ...) \
- do { \
- const auto listeners = gCtls->eventReporter.getNetdUnsolicitedEventListeners(); \
- for (auto& listener : listeners) { \
- auto entry = gUnsolicitedLog.newEntry().function(#func).args(__VA_ARGS__); \
- if (retry(listener->func(__VA_ARGS__))) { \
- gUnsolicitedLog.log(entry.withAutomaticDuration()); \
- } \
- } \
+#define LOG_EVENT_FUNC(retry, func, ...) \
+ do { \
+ const auto listenerMap = gCtls->eventReporter.getNetdUnsolicitedEventListenerMap(); \
+ for (auto& listener : listenerMap) { \
+ auto entry = gUnsolicitedLog.newEntry().function(#func).args(__VA_ARGS__); \
+ if (retry(listener.first->func(__VA_ARGS__))) { \
+ gUnsolicitedLog.log(entry.withAutomaticDuration()); \
+ } \
+ } \
} while (0)
namespace android {
diff --git a/server/binder/android/net/INetd.aidl b/server/binder/android/net/INetd.aidl
index 81f371e..db77d5f 100644
--- a/server/binder/android/net/INetd.aidl
+++ b/server/binder/android/net/INetd.aidl
@@ -1133,8 +1133,7 @@
/**
* Register unsolicited event listener
- * Netd supports multiple unsolicited event listeners, but only one per pid
- * A newer listener won't be registed if netd has an old one on the same pid.
+ * Netd supports multiple unsolicited event listeners.
*
* @param listener unsolicited event listener to register
* @throws ServiceSpecificException in case of failure, with an error code indicating the