Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9a54579a authored by Alex Buynytskyy's avatar Alex Buynytskyy
Browse files

Cleaning up resources on mount destruction. 

DataLoaderStub's lifetime is controlled externally, but we want to
release resources ASAP.

Bug: b/153874006
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: I34035f36d1fe4ed0e4916014d859feb7fe2c0a09
parent ab65cb18

core/java/android/service/dataloader/DataLoaderService.java

+1 −3
Original line number Diff line number Diff line
@@ -109,9 +109,7 @@ public abstract class DataLoaderService extends Service { 
                @NonNull IDataLoaderStatusListener listener)

                throws RuntimeException {

            try {

                if (!nativeCreateDataLoader(id, control, params, listener)) {

                    Slog.e(TAG, "Failed to create native loader for " + id);

                }

                nativeCreateDataLoader(id, control, params, listener);

            } catch (Exception ex) {

                Slog.e(TAG, "Failed to create native loader for " + id, ex);

                destroy(id);

services/incremental/IncrementalService.cpp

+26 −10
Original line number Diff line number Diff line
@@ -160,8 +160,8 @@ const bool IncrementalService::sEnablePerfLogging = 


IncrementalService::IncFsMount::~IncFsMount() {

    if (dataLoaderStub) {

        dataLoaderStub->requestDestroy();

        dataLoaderStub->waitForDestroy();

        dataLoaderStub->cleanupResources();

        dataLoaderStub = {};

    }

    LOG(INFO) << "Unmounting and cleaning up mount " << mountId << " with root '" << root << '\'';

    for (auto&& [target, _] : bindPoints) {
@@ -999,7 +999,8 @@ bool IncrementalService::startLoading(StorageId storage) const { 
            return false;

        }

    }

    return dataLoaderStub->requestStart();

    dataLoaderStub->requestStart();

    return true;

}



void IncrementalService::mountExistingImages() {
@@ -1466,11 +1467,17 @@ IncrementalService::DataLoaderStub::DataLoaderStub(IncrementalService& service, 
        mParams(std::move(params)),

        mControl(std::move(control)),

        mListener(externalListener ? *externalListener : DataLoaderStatusListener()) {

    //

}



IncrementalService::DataLoaderStub::~DataLoaderStub() {

    waitForDestroy();

IncrementalService::DataLoaderStub::~DataLoaderStub() = default;



void IncrementalService::DataLoaderStub::cleanupResources() {

    requestDestroy();

    mParams = {};

    mControl = {};

    waitForStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED, std::chrono::seconds(60));

    mListener = {};

    mId = kInvalidStorageId;

}



bool IncrementalService::DataLoaderStub::requestCreate() {
@@ -1485,10 +1492,6 @@ bool IncrementalService::DataLoaderStub::requestDestroy() { 
    return setTargetStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);

}



bool IncrementalService::DataLoaderStub::waitForDestroy(Clock::duration duration) {

    return waitForStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED, duration);

}



bool IncrementalService::DataLoaderStub::setTargetStatus(int status) {

    {

        std::unique_lock lock(mStatusMutex);
@@ -1541,6 +1544,10 @@ bool IncrementalService::DataLoaderStub::destroy() { 
}



bool IncrementalService::DataLoaderStub::fsmStep() {

    if (!isValid()) {

        return false;

    }



    int currentStatus;

    int targetStatus;

    {
@@ -1580,6 +1587,15 @@ bool IncrementalService::DataLoaderStub::fsmStep() { 
}



binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mountId, int newStatus) {

    if (!isValid()) {

        return binder::Status::

                fromServiceSpecificError(-EINVAL, "onStatusChange came to invalid DataLoaderStub");

    }

    if (mId != mountId) {

        LOG(ERROR) << "Mount ID mismatch: expected " << mId << ", but got: " << mountId;

        return binder::Status::fromServiceSpecificError(-EPERM, "Mount ID mismatch.");

    }



    {

        std::unique_lock lock(mStatusMutex);

        if (mCurrentStatus == newStatus) {

services/incremental/IncrementalService.h

+9 −6
Original line number Diff line number Diff line
@@ -173,13 +173,14 @@ private: 
                       FileSystemControlParcel&& control,

                       const DataLoaderStatusListener* externalListener);

        ~DataLoaderStub();

        // Cleans up the internal state and invalidates DataLoaderStub. Any subsequent calls will

        // result in an error.

        void cleanupResources();



        bool requestCreate();

        bool requestStart();

        bool requestDestroy();



        bool waitForDestroy(Clock::duration duration = std::chrono::seconds(60));



        void onDump(int fd);



        MountId id() const { return mId; }
@@ -188,6 +189,8 @@ private: 
    private:

        binder::Status onStatusChanged(MountId mount, int newStatus) final;



        bool isValid() const { return mId != kInvalidStorageId; }



        bool create();

        bool start();

        bool destroy();
@@ -198,10 +201,10 @@ private: 
        bool fsmStep();



        IncrementalService& mService;

        MountId const mId;

        DataLoaderParamsParcel const mParams;

        FileSystemControlParcel const mControl;

        DataLoaderStatusListener const mListener;

        MountId mId = kInvalidStorageId;

        DataLoaderParamsParcel mParams;

        FileSystemControlParcel mControl;

        DataLoaderStatusListener mListener;



        std::mutex mStatusMutex;

        std::condition_variable mStatusCondition;