From b887f68ad162578241050bb877cd7ae61c4876c8 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Thu, 15 Jul 2021 10:43:42 -0700 Subject: [PATCH] Use binder-based iterator to retrieve FRROs If for some reason there are a lot of fabricated overlays in the resources cache, the binder limit of the list of fabricated overlay infos could exceed the maximum binder transaction size. Rather than return all of the frro infos in one transactions, register an iterator with the native idmap2d service and use multiple binder transactions to iterate through all of the frros. Bug: 192948522 Test: Toggle device theme colors several times and observe frro cache Reboot device and observe old frros are deleted Change-Id: I5e9cf3ae9d1d45eda683c24141a0cd4e4301e02f --- cmds/idmap2/idmap2/CommandUtils.cpp | 3 +- cmds/idmap2/idmap2d/Idmap2Service.cpp | 35 +++++++++++++++---- cmds/idmap2/idmap2d/Idmap2Service.h | 9 +++-- .../aidl/services/android/os/IIdmap2.aidl | 7 +++- .../com/android/server/om/IdmapDaemon.java | 21 ++++++++--- 5 files changed, 61 insertions(+), 14 deletions(-) diff --git a/cmds/idmap2/idmap2/CommandUtils.cpp b/cmds/idmap2/idmap2/CommandUtils.cpp index 63235ff7d1998..d344d0f1c5311 100644 --- a/cmds/idmap2/idmap2/CommandUtils.cpp +++ b/cmds/idmap2/idmap2/CommandUtils.cpp @@ -14,12 +14,13 @@ * limitations under the License. */ +#include "idmap2/CommandUtils.h" + #include #include #include #include -#include "idmap2/CommandUtils.h" #include "idmap2/Idmap.h" #include "idmap2/Result.h" #include "idmap2/SysTrace.h" diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp index 2cfbac3f2c267..a8d648917b080 100644 --- a/cmds/idmap2/idmap2d/Idmap2Service.cpp +++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp @@ -297,17 +297,40 @@ Status Idmap2Service::createFabricatedOverlay( return ok(); } -Status Idmap2Service::getFabricatedOverlayInfos( +Status Idmap2Service::acquireFabricatedOverlayIterator() { + if (frro_iter_.has_value()) { + LOG(WARNING) << "active ffro iterator was not previously released"; + } + frro_iter_ = std::filesystem::directory_iterator(kIdmapCacheDir); + return ok(); +} + +Status Idmap2Service::releaseFabricatedOverlayIterator() { + if (!frro_iter_.has_value()) { + LOG(WARNING) << "no active ffro iterator to release"; + } + return ok(); +} + +Status Idmap2Service::nextFabricatedOverlayInfos( std::vector* _aidl_return) { - for (const auto& entry : std::filesystem::directory_iterator(kIdmapCacheDir)) { - if (!android::IsFabricatedOverlay(entry.path())) { + constexpr size_t kMaxEntryCount = 100; + if (!frro_iter_.has_value()) { + return error("no active frro iterator"); + } + + size_t count = 0; + auto& entry_iter = *frro_iter_; + auto entry_iter_end = end(*frro_iter_); + for (; entry_iter != entry_iter_end && count < kMaxEntryCount; ++entry_iter) { + auto& entry = *entry_iter; + if (!entry.is_regular_file() || !android::IsFabricatedOverlay(entry.path())) { continue; } const auto overlay = FabricatedOverlayContainer::FromPath(entry.path()); if (!overlay) { - // This is a sign something went wrong. - LOG(ERROR) << "Failed to open '" << entry.path() << "': " << overlay.GetErrorMessage(); + LOG(WARNING) << "Failed to open '" << entry.path() << "': " << overlay.GetErrorMessage(); continue; } @@ -319,8 +342,8 @@ Status Idmap2Service::getFabricatedOverlayInfos( out_info.targetOverlayable = info.target_name; out_info.path = entry.path(); _aidl_return->emplace_back(std::move(out_info)); + count++; } - return ok(); } diff --git a/cmds/idmap2/idmap2d/Idmap2Service.h b/cmds/idmap2/idmap2d/Idmap2Service.h index c16c3c52155f6..c61e4bc98a548 100644 --- a/cmds/idmap2/idmap2d/Idmap2Service.h +++ b/cmds/idmap2/idmap2d/Idmap2Service.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -59,7 +60,11 @@ class Idmap2Service : public BinderService, public BnIdmap2 { binder::Status deleteFabricatedOverlay(const std::string& overlay_path, bool* _aidl_return) override; - binder::Status getFabricatedOverlayInfos( + binder::Status acquireFabricatedOverlayIterator() override; + + binder::Status releaseFabricatedOverlayIterator() override; + + binder::Status nextFabricatedOverlayInfos( std::vector* _aidl_return) override; binder::Status dumpIdmap(const std::string& overlay_path, std::string* _aidl_return) override; @@ -69,7 +74,7 @@ class Idmap2Service : public BinderService, public BnIdmap2 { // be able to be recalculated if idmap2 dies and restarts. std::unique_ptr framework_apk_cache_; - std::vector fabricated_overlays_; + std::optional frro_iter_; template using MaybeUniquePtr = std::variant, T*>; diff --git a/cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl b/cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl index 48cee69ca7791..0059cf2931772 100644 --- a/cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl +++ b/cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl @@ -37,8 +37,13 @@ interface IIdmap2 { int fulfilledPolicies, boolean enforceOverlayable, int userId); + @nullable FabricatedOverlayInfo createFabricatedOverlay(in FabricatedOverlayInternal overlay); - List getFabricatedOverlayInfos(); boolean deleteFabricatedOverlay(@utf8InCpp String path); + + void acquireFabricatedOverlayIterator(); + void releaseFabricatedOverlayIterator(); + List nextFabricatedOverlayInfos(); + @utf8InCpp String dumpIdmap(@utf8InCpp String overlayApkPath); } diff --git a/services/core/java/com/android/server/om/IdmapDaemon.java b/services/core/java/com/android/server/om/IdmapDaemon.java index 555081a687266..c9e564a0ce9d9 100644 --- a/services/core/java/com/android/server/om/IdmapDaemon.java +++ b/services/core/java/com/android/server/om/IdmapDaemon.java @@ -36,6 +36,7 @@ import android.util.Slog; import com.android.server.FgThread; import java.io.File; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; @@ -161,13 +162,25 @@ class IdmapDaemon { } } - List getFabricatedOverlayInfos() { + synchronized List getFabricatedOverlayInfos() { + final ArrayList allInfos = new ArrayList<>(); try (Connection c = connect()) { - return mService.getFabricatedOverlayInfos(); + mService.acquireFabricatedOverlayIterator(); + List infos; + while (!(infos = mService.nextFabricatedOverlayInfos()).isEmpty()) { + allInfos.addAll(infos); + } + return allInfos; } catch (Exception e) { - Slog.wtf(TAG, "failed to get fabricated overlays", e); - return null; + Slog.wtf(TAG, "failed to get all fabricated overlays", e); + } finally { + try { + mService.releaseFabricatedOverlayIterator(); + } catch (RemoteException e) { + // ignore + } } + return allInfos; } String dumpIdmap(@NonNull String overlayPath) {