diff options
author | Ryan Mitchell <rtmitchell@google.com> | 2021-07-15 10:43:42 -0700 |
---|---|---|
committer | Ryan Mitchell <rtmitchell@google.com> | 2021-07-19 16:53:27 +0000 |
commit | b887f68ad162578241050bb877cd7ae61c4876c8 (patch) | |
tree | 3b156ce728ccf33110a79fab075620d8f16a6ae6 | |
parent | b827a8ac8354ad87aed758922291c22ee0504d85 (diff) |
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
-rw-r--r-- | cmds/idmap2/idmap2/CommandUtils.cpp | 3 | ||||
-rw-r--r-- | cmds/idmap2/idmap2d/Idmap2Service.cpp | 35 | ||||
-rw-r--r-- | cmds/idmap2/idmap2d/Idmap2Service.h | 9 | ||||
-rw-r--r-- | cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl | 7 | ||||
-rw-r--r-- | services/core/java/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 63235ff7d199..d344d0f1c531 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 <fstream> #include <memory> #include <string> #include <vector> -#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 2cfbac3f2c26..a8d648917b08 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<os::FabricatedOverlayInfo>* _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 c16c3c52155f..c61e4bc98a54 100644 --- a/cmds/idmap2/idmap2d/Idmap2Service.h +++ b/cmds/idmap2/idmap2d/Idmap2Service.h @@ -24,6 +24,7 @@ #include <idmap2/ResourceContainer.h> #include <idmap2/Result.h> +#include <filesystem> #include <memory> #include <string> #include <vector> @@ -59,7 +60,11 @@ class Idmap2Service : public BinderService<Idmap2Service>, 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<os::FabricatedOverlayInfo>* _aidl_return) override; binder::Status dumpIdmap(const std::string& overlay_path, std::string* _aidl_return) override; @@ -69,7 +74,7 @@ class Idmap2Service : public BinderService<Idmap2Service>, public BnIdmap2 { // be able to be recalculated if idmap2 dies and restarts. std::unique_ptr<idmap2::TargetResourceContainer> framework_apk_cache_; - std::vector<os::FabricatedOverlayInfo> fabricated_overlays_; + std::optional<std::filesystem::directory_iterator> frro_iter_; template <typename T> using MaybeUniquePtr = std::variant<std::unique_ptr<T>, T*>; diff --git a/cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl b/cmds/idmap2/idmap2d/aidl/services/android/os/IIdmap2.aidl index 48cee69ca779..0059cf293177 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<FabricatedOverlayInfo> getFabricatedOverlayInfos(); boolean deleteFabricatedOverlay(@utf8InCpp String path); + + void acquireFabricatedOverlayIterator(); + void releaseFabricatedOverlayIterator(); + List<FabricatedOverlayInfo> 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 555081a68726..c9e564a0ce9d 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<FabricatedOverlayInfo> getFabricatedOverlayInfos() { + synchronized List<FabricatedOverlayInfo> getFabricatedOverlayInfos() { + final ArrayList<FabricatedOverlayInfo> allInfos = new ArrayList<>(); try (Connection c = connect()) { - return mService.getFabricatedOverlayInfos(); + mService.acquireFabricatedOverlayIterator(); + List<FabricatedOverlayInfo> 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) { |