From 4c75673a121e055a258e8710de2a4ec9c263c68f Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 12 Jul 2019 14:18:37 -0700 Subject: Only shut down gsid when it has no more consumers. Consumers of gsiservice must now use IGsid for its top-level interface. IGsiService can be acquired through IGsid::getClient. When the last reference to IGsiService is dropped, gsid will cleanly exit. Callers should no longer stop gsid through init. Bug: 133528572 Test: gsid cleanly exits after gsi_tool runs Change-Id: Ie5cb80fa63e61b143f9f27cca96999a5c71dab2c Merged-In: Ie5cb80fa63e61b143f9f27cca96999a5c71dab2c --- Android.bp | 1 + aidl/android/gsi/IGsid.aidl | 30 +++++++++++++++++ daemon.cpp | 8 +++-- gsi_service.cpp | 80 ++++++++++++++++++++++++++++++--------------- gsi_service.h | 30 ++++++++++++----- gsi_tool.cpp | 14 ++++++-- gsid.rc | 1 + 7 files changed, 123 insertions(+), 41 deletions(-) create mode 100644 aidl/android/gsi/IGsid.aidl diff --git a/Android.bp b/Android.bp index 8eafb07..36631b0 100644 --- a/Android.bp +++ b/Android.bp @@ -100,6 +100,7 @@ filegroup { "aidl/android/gsi/GsiInstallParams.aidl", "aidl/android/gsi/GsiProgress.aidl", "aidl/android/gsi/IImageManager.aidl", + "aidl/android/gsi/IGsid.aidl", "aidl/android/gsi/IGsiService.aidl", "aidl/android/gsi/MappedImage.aidl", ], diff --git a/aidl/android/gsi/IGsid.aidl b/aidl/android/gsi/IGsid.aidl new file mode 100644 index 0000000..0c1a7b2 --- /dev/null +++ b/aidl/android/gsi/IGsid.aidl @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.gsi; + +import android.gsi.IGsiService; + +/** {@hide} */ +interface IGsid { + // Acquire an IGsiService client. gsid automatically shuts down when the + // last client is dropped. To start the daemon: + // + // 1. Check if the "init.svc.gsid" property is "running". If not, continue. + // 2. Set the "ctl.start" property to "gsid". + // 3. Wait for "init.svc.gsid" to be "running". + IGsiService getClient(); +} diff --git a/daemon.cpp b/daemon.cpp index 0ea1cb5..3fa7b3f 100644 --- a/daemon.cpp +++ b/daemon.cpp @@ -14,15 +14,17 @@ // limitations under the License. // -#include "gsi_service.h" - #include #include #include +#include #include #include +#include + +#include "gsi_service.h" using android::ProcessState; using android::sp; @@ -36,7 +38,7 @@ int main(int argc, char** argv) { exit(0); } - android::gsi::GsiService::Register(); + android::gsi::Gsid::Register(); { sp ps(ProcessState::self()); ps->startThreadPool(); diff --git a/gsi_service.cpp b/gsi_service.cpp index 18f1f4f..f0288f6 100644 --- a/gsi_service.cpp +++ b/gsi_service.cpp @@ -51,19 +51,44 @@ using namespace android::fiemap; using android::base::StringPrintf; using android::base::unique_fd; -void GsiService::Register() { - auto ret = android::BinderService::publish(); +android::wp GsiService::sInstance; + +void Gsid::Register() { + auto ret = android::BinderService::publish(); if (ret != android::OK) { LOG(FATAL) << "Could not register gsi service: " << ret; } } -GsiService::GsiService() { +binder::Status Gsid::getClient(android::sp* _aidl_return) { + *_aidl_return = GsiService::Get(this); + return binder::Status::ok(); +} + +GsiService::GsiService(Gsid* parent) : parent_(parent) { progress_ = {}; GsiInstaller::PostInstallCleanup(); } -GsiService::~GsiService() {} +GsiService::~GsiService() { + std::lock_guard guard(parent_->lock()); + + if (sInstance == this) { + // No more consumers, gracefully shut down gsid. + exit(0); + } +} + +android::sp GsiService::Get(Gsid* parent) { + std::lock_guard guard(parent->lock()); + + android::sp service = sInstance.promote(); + if (!service) { + service = new GsiService(parent); + sInstance = service.get(); + } + return service.get(); +} #define ENFORCE_SYSTEM \ do { \ @@ -89,7 +114,7 @@ binder::Status GsiService::startGsiInstall(int64_t gsiSize, int64_t userdataSize binder::Status GsiService::beginGsiInstall(const GsiInstallParams& given_params, int* _aidl_return) { ENFORCE_SYSTEM; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); // Make sure any interrupted installations are cleaned up. installer_ = nullptr; @@ -114,7 +139,7 @@ binder::Status GsiService::beginGsiInstall(const GsiInstallParams& given_params, binder::Status GsiService::commitGsiChunkFromStream(const android::os::ParcelFileDescriptor& stream, int64_t bytes, bool* _aidl_return) { ENFORCE_SYSTEM; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); if (!installer_) { *_aidl_return = false; @@ -156,7 +181,7 @@ binder::Status GsiService::getInstallProgress(::android::gsi::GsiProgress* _aidl binder::Status GsiService::commitGsiChunkFromMemory(const std::vector& bytes, bool* _aidl_return) { ENFORCE_SYSTEM; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); if (!installer_) { *_aidl_return = false; @@ -168,7 +193,7 @@ binder::Status GsiService::commitGsiChunkFromMemory(const std::vector& } binder::Status GsiService::setGsiBootable(bool one_shot, int* _aidl_return) { - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); if (installer_) { ENFORCE_SYSTEM; @@ -188,7 +213,7 @@ binder::Status GsiService::setGsiBootable(bool one_shot, int* _aidl_return) { binder::Status GsiService::isGsiEnabled(bool* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); std::string boot_key; if (!GetInstallStatus(&boot_key)) { *_aidl_return = false; @@ -200,7 +225,7 @@ binder::Status GsiService::isGsiEnabled(bool* _aidl_return) { binder::Status GsiService::removeGsiInstall(bool* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); // Just in case an install was left hanging. std::string install_dir; @@ -221,7 +246,7 @@ binder::Status GsiService::removeGsiInstall(bool* _aidl_return) { binder::Status GsiService::disableGsiInstall(bool* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); *_aidl_return = DisableGsiInstall(); return binder::Status::ok(); @@ -229,7 +254,7 @@ binder::Status GsiService::disableGsiInstall(bool* _aidl_return) { binder::Status GsiService::isGsiRunning(bool* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); *_aidl_return = IsGsiRunning(); return binder::Status::ok(); @@ -237,7 +262,7 @@ binder::Status GsiService::isGsiRunning(bool* _aidl_return) { binder::Status GsiService::isGsiInstalled(bool* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); *_aidl_return = IsGsiInstalled(); return binder::Status::ok(); @@ -245,7 +270,7 @@ binder::Status GsiService::isGsiInstalled(bool* _aidl_return) { binder::Status GsiService::isGsiInstallInProgress(bool* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); *_aidl_return = !!installer_; return binder::Status::ok(); @@ -254,7 +279,7 @@ binder::Status GsiService::isGsiInstallInProgress(bool* _aidl_return) { binder::Status GsiService::cancelGsiInstall(bool* _aidl_return) { ENFORCE_SYSTEM; should_abort_ = true; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); should_abort_ = false; installer_ = nullptr; @@ -265,7 +290,7 @@ binder::Status GsiService::cancelGsiInstall(bool* _aidl_return) { binder::Status GsiService::getGsiBootStatus(int* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); if (!IsGsiInstalled()) { *_aidl_return = BOOT_STATUS_NOT_INSTALLED; @@ -304,7 +329,7 @@ binder::Status GsiService::getGsiBootStatus(int* _aidl_return) { binder::Status GsiService::getUserdataImageSize(int64_t* _aidl_return) { ENFORCE_SYSTEM; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); *_aidl_return = -1; @@ -337,7 +362,7 @@ binder::Status GsiService::getUserdataImageSize(int64_t* _aidl_return) { binder::Status GsiService::getInstalledGsiImageDir(std::string* _aidl_return) { ENFORCE_SYSTEM; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); if (IsGsiInstalled()) { *_aidl_return = GetInstalledImageDir(); @@ -347,7 +372,7 @@ binder::Status GsiService::getInstalledGsiImageDir(std::string* _aidl_return) { binder::Status GsiService::wipeGsiUserdata(int* _aidl_return) { ENFORCE_SYSTEM_OR_SHELL; - std::lock_guard guard(lock_); + std::lock_guard guard(parent_->lock()); if (IsGsiRunning() || !IsGsiInstalled()) { *_aidl_return = IGsiService::INSTALL_ERROR_GENERIC; @@ -373,7 +398,7 @@ static binder::Status UidSecurityError() { class ImageManagerService : public BinderService, public BnImageManager { public: - ImageManagerService(GsiService* parent, std::unique_ptr&& impl, uid_t uid); + ImageManagerService(GsiService* service, std::unique_ptr&& impl, uid_t uid); binder::Status createBackingImage(const std::string& name, int64_t size, int flags) override; binder::Status deleteBackingImage(const std::string& name) override; binder::Status mapImageDevice(const std::string& name, int32_t timeout_ms, @@ -383,20 +408,21 @@ class ImageManagerService : public BinderService, public Bn private: bool CheckUid(); - android::sp parent_; + android::sp service_; + android::sp parent_; std::unique_ptr impl_; uid_t uid_; }; -ImageManagerService::ImageManagerService(GsiService* parent, std::unique_ptr&& impl, +ImageManagerService::ImageManagerService(GsiService* service, std::unique_ptr&& impl, uid_t uid) - : parent_(parent), impl_(std::move(impl)), uid_(uid) {} + : service_(service), parent_(service->parent()), impl_(std::move(impl)), uid_(uid) {} binder::Status ImageManagerService::createBackingImage(const std::string& name, int64_t size, int flags) { if (!CheckUid()) return UidSecurityError(); - std::lock_guard guard(*parent_->lock()); + std::lock_guard guard(parent_->lock()); if (!impl_->CreateBackingImage(name, size, flags, nullptr)) { return BinderError("Failed to create"); @@ -407,7 +433,7 @@ binder::Status ImageManagerService::createBackingImage(const std::string& name, binder::Status ImageManagerService::deleteBackingImage(const std::string& name) { if (!CheckUid()) return UidSecurityError(); - std::lock_guard guard(*parent_->lock()); + std::lock_guard guard(parent_->lock()); if (!impl_->DeleteBackingImage(name)) { return BinderError("Failed to delete"); @@ -419,7 +445,7 @@ binder::Status ImageManagerService::mapImageDevice(const std::string& name, int3 MappedImage* mapping) { if (!CheckUid()) return UidSecurityError(); - std::lock_guard guard(*parent_->lock()); + std::lock_guard guard(parent_->lock()); if (!impl_->MapImageDevice(name, std::chrono::milliseconds(timeout_ms), &mapping->path)) { return BinderError("Failed to map"); @@ -430,7 +456,7 @@ binder::Status ImageManagerService::mapImageDevice(const std::string& name, int3 binder::Status ImageManagerService::unmapImageDevice(const std::string& name) { if (!CheckUid()) return UidSecurityError(); - std::lock_guard guard(*parent_->lock()); + std::lock_guard guard(parent_->lock()); if (!impl_->UnmapImageDevice(name)) { return BinderError("Failed to unmap"); diff --git a/gsi_service.h b/gsi_service.h index ad582dd..6d72921 100644 --- a/gsi_service.h +++ b/gsi_service.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -34,13 +35,28 @@ namespace android { namespace gsi { -class GsiService : public BinderService, public BnGsiService { +class Gsid : public BinderService, public BnGsid { public: static void Register(); + static char const* getServiceName() { return kGsiServiceName; } + + binder::Status getClient(android::sp* _aidl_return) override; + + private: + friend class GsiService; + friend class ImageManagerService; + + std::mutex& lock() { return lock_; } + + std::mutex lock_; +}; - GsiService(); +class GsiService : public BinderService, public BnGsiService { + public: ~GsiService() override; + static android::sp Get(Gsid* parent); + binder::Status startGsiInstall(int64_t gsiSize, int64_t userdataSize, bool wipeUserdata, int* _aidl_return) override; binder::Status beginGsiInstall(const GsiInstallParams& params, int* _aidl_return) override; @@ -69,18 +85,16 @@ class GsiService : public BinderService, public BnGsiService { void StartAsyncOperation(const std::string& step, int64_t total_bytes); void UpdateProgress(int status, int64_t bytes_processed); - static char const* getServiceName() { return kGsiServiceName; } - // Helper methods for GsiInstaller. static bool RemoveGsiFiles(const std::string& install_dir, bool wipeUserdata); bool should_abort() const { return should_abort_; } + Gsid* parent() const { return parent_.get(); } static void RunStartupTasks(); static std::string GetInstalledImageDir(); private: - friend class ImageManagerService; - + GsiService(Gsid* parent); int ValidateInstallParams(GsiInstallParams* params); bool DisableGsiInstall(); int ReenableGsi(bool one_shot); @@ -88,9 +102,9 @@ class GsiService : public BinderService, public BnGsiService { enum class AccessLevel { System, SystemOrShell }; binder::Status CheckUid(AccessLevel level = AccessLevel::System); - std::mutex* lock() { return &lock_; } + static android::wp sInstance; - std::mutex lock_; + android::sp parent_; std::unique_ptr installer_; // These are initialized or set in StartInstall(). diff --git a/gsi_tool.cpp b/gsi_tool.cpp index f5f549f..c33e645 100644 --- a/gsi_tool.cpp +++ b/gsi_tool.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -61,7 +62,7 @@ static const std::map kCommandMap = { // clang-format on }; -static sp GetGsiService() { +static sp GetGsiService() { if (android::base::GetProperty("init.svc.gsid", "") != "running") { if (!android::base::SetProperty("ctl.start", "gsid") || !android::base::WaitForProperty("init.svc.gsid", "running", 5s)) { @@ -77,7 +78,7 @@ static sp GetGsiService() { auto name = android::String16(kGsiServiceName); android::sp res = sm->checkService(name); if (res) { - return android::interface_cast(res); + return android::interface_cast(res); } usleep(kSleepTimeMs * 1000); } @@ -515,6 +516,13 @@ int main(int argc, char** argv) { return EX_NOPERM; } + android::sp service; + auto status = gsid->getClient(&service); + if (!status.isOk()) { + std::cerr << "Could not get gsi client: " << ErrorMessage(status) << "\n"; + return EX_SOFTWARE; + } + if (1 >= argc) { std::cerr << "Expected command." << std::endl; return EX_USAGE; @@ -528,6 +536,6 @@ int main(int argc, char** argv) { return usage(argc, argv); } - int rc = iter->second(gsid, argc - 1, argv + 1); + int rc = iter->second(service, argc - 1, argv + 1); return rc; } diff --git a/gsid.rc b/gsid.rc index a5248c4..2831df2 100644 --- a/gsid.rc +++ b/gsid.rc @@ -1,4 +1,5 @@ service gsid /system/bin/gsid + oneshot disabled user root group root system media_rw -- cgit v1.2.3