Merge "Do not destruct static maps." into qt-dev

7 files changed, 50 insertions, 25 deletions

diff --git a/transport/HidlBinderSupport.cpp b/transport/HidlBinderSupport.cpp
index caf7cf8..62b1755 100644
--- a/transport/HidlBinderSupport.cpp
+++ b/transport/HidlBinderSupport.cpp
@@ -256,16 +256,16 @@ sp<IBinder> getOrCreateCachedBinder(::android::hidl::base::V1_0::IBase* ifacePtr
     }
 
     // for get + set
-    std::unique_lock<std::mutex> _lock = details::gBnMap.lock();
+    std::unique_lock<std::mutex> _lock = details::gBnMap->lock();
 
-    wp<BHwBinder> wBnObj = details::gBnMap.getLocked(ifacePtr, nullptr);
+    wp<BHwBinder> wBnObj = details::gBnMap->getLocked(ifacePtr, nullptr);
     sp<IBinder> sBnObj = wBnObj.promote();
 
     if (sBnObj == nullptr) {
         auto func = details::getBnConstructorMap().get(descriptor, nullptr);
         if (!func) {
             // TODO(b/69122224): remove this static variable when prebuilts updated
-            func = details::gBnConstructorMap.get(descriptor, nullptr);
+            func = details::gBnConstructorMap->get(descriptor, nullptr);
         }
         LOG_ALWAYS_FATAL_IF(func == nullptr, "%s gBnConstructorMap returned null for %s", __func__,
                             descriptor.c_str());
@@ -274,7 +274,7 @@ sp<IBinder> getOrCreateCachedBinder(::android::hidl::base::V1_0::IBase* ifacePtr
         LOG_ALWAYS_FATAL_IF(sBnObj == nullptr, "%s Bn constructor function returned null for %s",
                             __func__, descriptor.c_str());
 
-        details::gBnMap.setLocked(ifacePtr, static_cast<BHwBinder*>(sBnObj.get()));
+        details::gBnMap->setLocked(ifacePtr, static_cast<BHwBinder*>(sBnObj.get()));
     }
 
     return sBnObj;
diff --git a/transport/HidlPassthroughSupport.cpp b/transport/HidlPassthroughSupport.cpp
index ff68a1e..bc67656 100644
--- a/transport/HidlPassthroughSupport.cpp
+++ b/transport/HidlPassthroughSupport.cpp
@@ -31,7 +31,7 @@ static sp<IBase> tryWrap(const std::string& descriptor, sp<IBase> iface) {
     auto func = getBsConstructorMap().get(descriptor, nullptr);
     if (!func) {
         // TODO(b/69122224): remove this when prebuilts don't reference it
-        func = gBsConstructorMap.get(descriptor, nullptr);
+        func = gBsConstructorMap->get(descriptor, nullptr);
     }
     if (func) {
         return func(static_cast<void*>(iface.get()));
diff --git a/transport/HidlTransportSupport.cpp b/transport/HidlTransportSupport.cpp
index db70438..b433b70 100644
--- a/transport/HidlTransportSupport.cpp
+++ b/transport/HidlTransportSupport.cpp
@@ -85,9 +85,9 @@ bool setMinSchedulerPolicy(const sp<IBase>& service, int policy, int priority) {
     // Due to ABI considerations, IBase cannot have a destructor to clean this up.
     // So, because this API is so infrequently used, (expected to be usually only
     // one time for a process, but it can be more), we are cleaning it up here.
-    std::unique_lock<std::mutex> lock = details::gServicePrioMap.lock();
-    pruneMapLocked(details::gServicePrioMap);
-    details::gServicePrioMap.setLocked(service, {policy, priority});
+    std::unique_lock<std::mutex> lock = details::gServicePrioMap->lock();
+    pruneMapLocked(details::gServicePrioMap.get());
+    details::gServicePrioMap->setLocked(service, {policy, priority});
 
     return true;
 }
@@ -101,9 +101,9 @@ bool setRequestingSid(const sp<IBase>& service, bool requesting) {
     // Due to ABI considerations, IBase cannot have a destructor to clean this up.
     // So, because this API is so infrequently used, (expected to be usually only
     // one time for a process, but it can be more), we are cleaning it up here.
-    std::unique_lock<std::mutex> lock = details::gServiceSidMap.lock();
-    pruneMapLocked(details::gServiceSidMap);
-    details::gServiceSidMap.setLocked(service, requesting);
+    std::unique_lock<std::mutex> lock = details::gServiceSidMap->lock();
+    pruneMapLocked(details::gServiceSidMap.get());
+    details::gServiceSidMap->setLocked(service, requesting);
 
     return true;
 }
diff --git a/transport/InternalStatic.h b/transport/InternalStatic.h
index b0fefa9..1dfaae4 100644
--- a/transport/InternalStatic.h
+++ b/transport/InternalStatic.h
@@ -26,10 +26,12 @@ namespace android {
 namespace hardware {
 namespace details {
 
+// TODO(b/69122224): remove this
 // deprecated; use getBnConstructorMap instead.
-extern BnConstructorMap gBnConstructorMap;
+extern DoNotDestruct<BnConstructorMap> gBnConstructorMap;
+// TODO(b/69122224): remove this
 // deprecated; use getBsConstructorMap instead.
-extern BsConstructorMap gBsConstructorMap;
+extern DoNotDestruct<BsConstructorMap> gBsConstructorMap;
 
 }  // namespace details
 }  // namespace hardware
diff --git a/transport/Static.cpp b/transport/Static.cpp
index 0bbd48d..af16e8f 100644
--- a/transport/Static.cpp
+++ b/transport/Static.cpp
@@ -28,27 +28,28 @@ namespace hardware {
 namespace details {
 
 // Deprecated; kept for ABI compatibility. Use getBnConstructorMap.
-BnConstructorMap gBnConstructorMap{};
+DoNotDestruct<BnConstructorMap> gBnConstructorMap{};
 
-ConcurrentMap<const ::android::hidl::base::V1_0::IBase*, wp<::android::hardware::BHwBinder>>
-    gBnMap{};
+DoNotDestruct<ConcurrentMap<const ::android::hidl::base::V1_0::IBase*,
+                            wp<::android::hardware::BHwBinder>>>
+        gBnMap{};
 
 // TODO(b/122472540): replace with single, hidden map
-ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, SchedPrio> gServicePrioMap{};
-ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, bool> gServiceSidMap{};
+DoNotDestruct<ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, SchedPrio>> gServicePrioMap{};
+DoNotDestruct<ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, bool>> gServiceSidMap{};
 
 // Deprecated; kept for ABI compatibility. Use getBsConstructorMap.
-BsConstructorMap gBsConstructorMap{};
+DoNotDestruct<BsConstructorMap> gBsConstructorMap{};
 
 // For static executables, it is not guaranteed that gBnConstructorMap are initialized before
 // used in HAL definition libraries.
 BnConstructorMap& getBnConstructorMap() {
-    static BnConstructorMap map{};
+    static BnConstructorMap& map = *new BnConstructorMap();
     return map;
 }
 
 BsConstructorMap& getBsConstructorMap() {
-    static BsConstructorMap map{};
+    static BsConstructorMap& map = *new BsConstructorMap();
     return map;
 }
 
diff --git a/transport/include/hidl/ConcurrentMap.h b/transport/include/hidl/ConcurrentMap.h
index 1b06dfd..329752c 100644
--- a/transport/include/hidl/ConcurrentMap.h
+++ b/transport/include/hidl/ConcurrentMap.h
@@ -90,6 +90,23 @@ public:
     std::map<K, V> mMap;
 };
 
+namespace details {
+
+// TODO(b/69122224): remove this type and usages of it
+// DO NOT ADD USAGES
+template <typename T>
+class DoNotDestruct {
+  public:
+    DoNotDestruct() { new (buffer) T(); }
+    T& get() { return *reinterpret_cast<T*>(buffer); }
+    T* operator->() { return reinterpret_cast<T*>(buffer); }
+
+  private:
+    alignas(T) char buffer[sizeof(T)];
+};
+
+}  // namespace details
+
 }  // namespace hardware
 }  // namespace android
 
diff --git a/transport/include/hidl/Static.h b/transport/include/hidl/Static.h
index cc711b7..be74bae 100644
--- a/transport/include/hidl/Static.h
+++ b/transport/include/hidl/Static.h
@@ -37,12 +37,17 @@ struct SchedPrio {
     int prio;
 };
 
-extern ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, SchedPrio> gServicePrioMap;
-extern ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, bool> gServiceSidMap;
+// TODO(b/69122224): remove this
+extern DoNotDestruct<ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, SchedPrio>>
+        gServicePrioMap;
+// TODO(b/69122224): remove this
+extern DoNotDestruct<ConcurrentMap<wp<::android::hidl::base::V1_0::IBase>, bool>> gServiceSidMap;
 
+// TODO(b/69122224): remove this
 // For HidlBinderSupport and autogenerated code
-extern ConcurrentMap<const ::android::hidl::base::V1_0::IBase*, wp<::android::hardware::BHwBinder>>
-    gBnMap;
+extern DoNotDestruct<ConcurrentMap<const ::android::hidl::base::V1_0::IBase*,
+                                   wp<::android::hardware::BHwBinder>>>
+        gBnMap;
 
 using BnConstructorMap = ConcurrentMap<std::string, std::function<sp<IBinder>(void*)>>;
 // For HidlBinderSupport and autogenerated code