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

Commit 1c682406 authored by Girish's avatar Girish
Browse files

resourcemanager: fix reclaim issue with conflicting codecs 

If a device doesn't support multiple secure codecs,
the resource manager has to resolve the conflicts while
reclaiming.
But when a secure codec fails during configure or start with
insufficient resources, the RM thinks this as secure codec
conflict, as the resources associated with this secure codec
was added during creation (init) time.
This happens as RM only checks for the calling/requesting process
against the currently added resources.
The fix is to make sure that the calling/requesting client
is compared with other clients only.

Bug: 323238132
Test: atest android.media.misc.cts.ResourceManagerTest
      atest android.media.misc.cts.ResourceManagerMultiTest
      /data/nativetest64/ResourceManagerService_test/ResourceManagerService_test
      /data/nativetest64/ResourceObserverService_test/ResourceObserverService_test
Change-Id: Ieff5275abb5724bc2b8eb27babd366645f7db3d9
parent a0934756

services/mediaresourcemanager/DefaultResourceModel.cpp

+6 −2
Original line number Diff line number Diff line
@@ -44,7 +44,9 @@ bool DefaultResourceModel::getAllClients( 
    clients.clear();

    MediaResourceParcel mediaResource{.type = reclimRequestInfo.mResources[0].type,

                                      .subType = reclimRequestInfo.mResources[0].subType};

    ResourceRequestInfo resourceRequestInfo{reclimRequestInfo.mCallingPid, &mediaResource};

    ResourceRequestInfo resourceRequestInfo{reclimRequestInfo.mCallingPid,

                                            reclimRequestInfo.mClientId,

                                            &mediaResource};



    // Resolve the secure-unsecure codec conflicts if there is any.

    switch (reclimRequestInfo.mResources[0].type) {
@@ -116,7 +118,9 @@ bool DefaultResourceModel::getCodecClients( 
        const ReclaimRequestInfo& reclimRequestInfo,

        std::vector<ClientInfo>& clients) {

    MediaResourceParcel mediaResource;

    ResourceRequestInfo resourceRequestInfo{reclimRequestInfo.mCallingPid, &mediaResource};

    ResourceRequestInfo resourceRequestInfo{reclimRequestInfo.mCallingPid,

                                            reclimRequestInfo.mClientId,

                                            &mediaResource};



    // 1. Look to find the client(s) with the other resources, for the given

    // primary type.

services/mediaresourcemanager/ResourceManagerService.cpp

+14 −8
Original line number Diff line number Diff line
@@ -474,6 +474,7 @@ bool ResourceManagerService::getTargetClients( 
        const std::vector<MediaResourceParcel>& resources,

        std::vector<ClientInfo>& targetClients) {

    int32_t callingPid = clientInfo.pid;

    int64_t clientId = clientInfo.id;

    std::scoped_lock lock{mLock};

    if (!mProcessInfo->isPidTrusted(callingPid)) {

        pid_t actualCallingPid = IPCThreadState::self()->getCallingPid();
@@ -508,7 +509,7 @@ bool ResourceManagerService::getTargetClients( 
    if (secureCodec != NULL) {

        MediaResourceParcel mediaResource{.type = MediaResource::Type::kSecureCodec,

                                          .subType = secureCodec->subType};

        ResourceRequestInfo resourceRequestInfo{callingPid, &mediaResource};

        ResourceRequestInfo resourceRequestInfo{callingPid, clientId, &mediaResource};

        if (!mSupportsMultipleSecureCodecs) {

            if (!getAllClients_l(resourceRequestInfo, targetClients)) {

                return false;
@@ -525,7 +526,7 @@ bool ResourceManagerService::getTargetClients( 
        if (!mSupportsSecureWithNonSecureCodec) {

            MediaResourceParcel mediaResource{.type = MediaResource::Type::kSecureCodec,

                                              .subType = nonSecureCodec->subType};

            ResourceRequestInfo resourceRequestInfo{callingPid, &mediaResource};

            ResourceRequestInfo resourceRequestInfo{callingPid, clientId, &mediaResource};

            if (!getAllClients_l(resourceRequestInfo, targetClients)) {

                return false;

            }
@@ -533,7 +534,7 @@ bool ResourceManagerService::getTargetClients( 
    }



    if (drmSession != NULL) {

        ResourceRequestInfo resourceRequestInfo{callingPid, drmSession};

        ResourceRequestInfo resourceRequestInfo{callingPid, clientId, drmSession};

        getClientForResource_l(resourceRequestInfo, targetClients);

        if (targetClients.size() == 0) {

            return false;
@@ -542,18 +543,18 @@ bool ResourceManagerService::getTargetClients( 


    if (targetClients.size() == 0 && graphicMemory != nullptr) {

        // if no secure/non-secure codec conflict, run second pass to handle other resources.

        ResourceRequestInfo resourceRequestInfo{callingPid, graphicMemory};

        ResourceRequestInfo resourceRequestInfo{callingPid, clientId, graphicMemory};

        getClientForResource_l(resourceRequestInfo, targetClients);

    }



    if (targetClients.size() == 0) {

        // if we are here, run the third pass to free one codec with the same type.

        if (secureCodec != nullptr) {

            ResourceRequestInfo resourceRequestInfo{callingPid, secureCodec};

            ResourceRequestInfo resourceRequestInfo{callingPid, clientId, secureCodec};

            getClientForResource_l(resourceRequestInfo, targetClients);

        }

        if (nonSecureCodec != nullptr) {

            ResourceRequestInfo resourceRequestInfo{callingPid, nonSecureCodec};

            ResourceRequestInfo resourceRequestInfo{callingPid, clientId, nonSecureCodec};

            getClientForResource_l(resourceRequestInfo, targetClients);

        }

    }
@@ -562,12 +563,12 @@ bool ResourceManagerService::getTargetClients( 
        // if we are here, run the fourth pass to free one codec with the different type.

        if (secureCodec != nullptr) {

            MediaResource temp(MediaResource::Type::kNonSecureCodec, secureCodec->subType, 1);

            ResourceRequestInfo resourceRequestInfo{callingPid, &temp};

            ResourceRequestInfo resourceRequestInfo{callingPid, clientId, &temp};

            getClientForResource_l(resourceRequestInfo, targetClients);

        }

        if (nonSecureCodec != nullptr) {

            MediaResource temp(MediaResource::Type::kSecureCodec, nonSecureCodec->subType, 1);

            ResourceRequestInfo resourceRequestInfo{callingPid, &temp};

            ResourceRequestInfo resourceRequestInfo{callingPid, clientId, &temp};

            getClientForResource_l(resourceRequestInfo, targetClients);

        }

    }
@@ -914,6 +915,11 @@ bool ResourceManagerService::getAllClients_l( 


    for (auto& [pid, infos] : mMap) {

        for (const auto& [id, info] : infos) {

            if (pid == resourceRequestInfo.mCallingPid && id == resourceRequestInfo.mClientId) {

                ALOGI("%s: Skip the client[%jd] for which the resource request is made",

                      __func__, id);

                continue;

            }

            if (hasResourceType(type, subType, info.resources)) {

                if (!isCallingPriorityHigher_l(resourceRequestInfo.mCallingPid, pid)) {

                    // some higher/equal priority process owns the resource,

services/mediaresourcemanager/ResourceManagerServiceNew.cpp

+5 −2
Original line number Diff line number Diff line
@@ -248,7 +248,7 @@ bool ResourceManagerServiceNew::getTargetClients( 
    // Use the Resource Model to get a list of all the clients that hold the

    // needed/requested resources.

    uint32_t callingImportance = std::max(0, clientInfo.importance);

    ReclaimRequestInfo reclaimRequestInfo{callingPid, callingImportance, resources};

    ReclaimRequestInfo reclaimRequestInfo{callingPid, clientInfo.id, callingImportance, resources};

    std::vector<ClientInfo> clients;

    if (!mDefaultResourceModel->getAllClients(reclaimRequestInfo, clients)) {

        if (clients.empty()) {
@@ -300,7 +300,10 @@ bool ResourceManagerServiceNew::getLowestPriorityBiggestClient_l( 


    // Use the DefaultResourceModel to get all the clients with the resources requested.

    std::vector<MediaResourceParcel> resources{*resourceRequestInfo.mResource};

    ReclaimRequestInfo reclaimRequestInfo{resourceRequestInfo.mCallingPid, 0, resources};

    ReclaimRequestInfo reclaimRequestInfo{resourceRequestInfo.mCallingPid,

                                          resourceRequestInfo.mClientId,

                                          0, // default importance

                                          resources};

    std::vector<ClientInfo> clients;

    mDefaultResourceModel->getAllClients(reclaimRequestInfo, clients);

services/mediaresourcemanager/ResourceManagerServiceUtils.h

+5 −0
Original line number Diff line number Diff line
@@ -166,11 +166,13 @@ struct ResourceInfo { 
/*

 * Resource Reclaim request info that encapsulates

 *  - the calling/requesting process pid.

 *  - id of the client that made reclaim request.

 *  - the calling/requesting client's importance.

 *  - the list of resources requesting (to be reclaimed from others)

 */

struct ReclaimRequestInfo {

    int mCallingPid = -1;

    int64_t mClientId = 0;

    uint32_t mCallingClientImportance = 0;

    const std::vector<::aidl::android::media::MediaResourceParcel>& mResources;

};
@@ -178,11 +180,14 @@ struct ReclaimRequestInfo { 
/*

 * Resource request info that encapsulates

 *  - the calling/requesting process pid.

 *  - the calling/requesting client's id.

 *  - the resource requesting (to be reclaimed from others)

 */

struct ResourceRequestInfo {

    // pid of the calling/requesting process.

    int mCallingPid = -1;

    // id of the calling/requesting client.

    int64_t mClientId = 0;

    // resources requested.

    const ::aidl::android::media::MediaResourceParcel* mResource;

};

services/mediaresourcemanager/ResourceTracker.cpp

+5 −0
Original line number Diff line number Diff line
@@ -715,6 +715,11 @@ bool ResourceTracker::getNonConflictingClients(const ResourceRequestInfo& resour 
    MediaResource::SubType subType = resourceRequestInfo.mResource->subType;

    for (auto& [pid, /* ResourceInfos */ infos] : mMap) {

        for (const auto& [id, /* ResourceInfo */ info] : infos) {

            if (pid == resourceRequestInfo.mCallingPid && id == resourceRequestInfo.mClientId) {

                ALOGI("%s: Skip the client[%jd] for which the resource request is made",

                      __func__, id);

                continue;

            }

            if (hasResourceType(type, subType, info.resources)) {

                if (!isCallingPriorityHigher(resourceRequestInfo.mCallingPid, pid)) {

                    // some higher/equal priority process owns the resource,