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

Commit 3fa408f8 authored by Chong Zhang's avatar Chong Zhang
Browse files

transcoding: fixes for binder died handling 

Change client id type to uintptr_t counter, instead of
casting binder to int64_t.

Save all shared_ptrs of clients in global registry.

bug: 154734285
bug: 145233472

test: transcoding unit tests; manually plant crash
in test client and check binderDied handling in log.
Change-Id: If6523a1c6b7ce797a2201462399174e9cf0a3c71

Change-Id: I6b312c0f28a345285e27b738c37baee2bccae755
parent df797540

media/libmediatranscoding/TranscodingClientManager.cpp

+51 −15
Original line number Diff line number Diff line
@@ -32,6 +32,13 @@ using ::aidl::android::media::TranscodingRequestParcel; 
using Status = ::ndk::ScopedAStatus;

using ::ndk::SpAIBinder;



//static

std::atomic<ClientIdType> TranscodingClientManager::sCookieCounter = 0;

//static

std::mutex TranscodingClientManager::sCookie2ClientLock;

//static

std::map<ClientIdType, std::shared_ptr<TranscodingClientManager::ClientImpl>>

        TranscodingClientManager::sCookie2Client;

///////////////////////////////////////////////////////////////////////////////



/**
@@ -56,7 +63,7 @@ struct TranscodingClientManager::ClientImpl : public BnTranscodingClient { 
    std::string mClientOpPackageName;



    // Next jobId to assign

    std::atomic<std::int32_t> mNextJobId;

    std::atomic<int32_t> mNextJobId;

    // Pointer to the client manager for this client

    TranscodingClientManager* mOwner;
@@ -81,7 +88,7 @@ TranscodingClientManager::ClientImpl::ClientImpl( 
        TranscodingClientManager* owner)

      : mClientBinder((callback != nullptr) ? callback->asBinder() : nullptr),

        mClientCallback(callback),

        mClientId((int64_t)mClientBinder.get()),

        mClientId(sCookieCounter.fetch_add(1, std::memory_order_relaxed)),

        mClientPid(pid),

        mClientUid(uid),

        mClientName(clientName),
@@ -142,10 +149,25 @@ Status TranscodingClientManager::ClientImpl::unregister() { 


// static

void TranscodingClientManager::BinderDiedCallback(void* cookie) {

    ClientImpl* client = static_cast<ClientImpl*>(cookie);

    ALOGD("Client %lld is dead", (long long)client->mClientId);

    ClientIdType clientId = reinterpret_cast<ClientIdType>(cookie);



    ALOGD("Client %lld is dead", (long long)clientId);



    std::shared_ptr<ClientImpl> client;



    {

        std::scoped_lock lock{sCookie2ClientLock};



        auto it = sCookie2Client.find(clientId);

        if (it != sCookie2Client.end()) {

            client = it->second;

        }

    }



    if (client != nullptr) {

        client->unregister();

    }

}



TranscodingClientManager::TranscodingClientManager(

        const std::shared_ptr<SchedulerClientInterface>& scheduler)
@@ -191,25 +213,33 @@ status_t TranscodingClientManager::addClient( 
        return BAD_VALUE;

    }



    // Creates the client and uses its process id as client id.

    std::shared_ptr<ClientImpl> client = ::ndk::SharedRefBase::make<ClientImpl>(

            callback, pid, uid, clientName, opPackageName, this);

    SpAIBinder binder = callback->asBinder();



    std::scoped_lock lock{mLock};



    // Checks if the client already registers.

    if (mClientIdToClientMap.find(client->mClientId) != mClientIdToClientMap.end()) {

    if (mRegisteredCallbacks.count((uintptr_t)binder.get()) > 0) {

        return ALREADY_EXISTS;

    }



    // Creates the client and uses its process id as client id.

    std::shared_ptr<ClientImpl> client = ::ndk::SharedRefBase::make<ClientImpl>(

            callback, pid, uid, clientName, opPackageName, this);



    ALOGD("Adding client id %lld, pid %d, uid %d, name %s, package %s",

          (long long)client->mClientId, client->mClientPid, client->mClientUid,

          client->mClientName.c_str(), client->mClientOpPackageName.c_str());



    AIBinder_linkToDeath(client->mClientBinder.get(), mDeathRecipient.get(),

                         reinterpret_cast<void*>(client.get()));

    {

        std::scoped_lock lock{sCookie2ClientLock};

        sCookie2Client.emplace(std::make_pair(client->mClientId, client));

    }



    AIBinder_linkToDeath(binder.get(), mDeathRecipient.get(),

                         reinterpret_cast<void*>(client->mClientId));



    // Adds the new client to the map.

    mRegisteredCallbacks.insert((uintptr_t)binder.get());

    mClientIdToClientMap[client->mClientId] = client;



    *outClient = client;
@@ -228,16 +258,22 @@ status_t TranscodingClientManager::removeClient(ClientIdType clientId) { 
        return INVALID_OPERATION;

    }



    SpAIBinder callback = it->second->mClientBinder;

    SpAIBinder binder = it->second->mClientBinder;



    // Check if the client still live. If alive, unlink the death.

    if (callback.get() != nullptr) {

        AIBinder_unlinkToDeath(callback.get(), mDeathRecipient.get(),

                               reinterpret_cast<void*>(it->second.get()));

    if (binder.get() != nullptr) {

        AIBinder_unlinkToDeath(binder.get(), mDeathRecipient.get(),

                               reinterpret_cast<void*>(it->second->mClientId));

    }



    {

        std::scoped_lock lock{sCookie2ClientLock};

        sCookie2Client.erase(it->second->mClientId);

    }



    // Erase the entry.

    mClientIdToClientMap.erase(it);

    mRegisteredCallbacks.erase((uintptr_t)binder.get());



    return OK;

}

media/libmediatranscoding/TranscodingJobScheduler.cpp

+8 −6
Original line number Diff line number Diff line
@@ -174,7 +174,7 @@ void TranscodingJobScheduler::moveUidsToTop_l(const std::unordered_set<uid_t>& u 
    }

}



bool TranscodingJobScheduler::submit(ClientIdType clientId, int32_t jobId, uid_t uid,

bool TranscodingJobScheduler::submit(ClientIdType clientId, JobIdType jobId, uid_t uid,

                                     const TranscodingRequestParcel& request,

                                     const std::weak_ptr<ITranscodingClientCallback>& callback) {

    JobKeyType jobKey = std::make_pair(clientId, jobId);
@@ -231,7 +231,7 @@ bool TranscodingJobScheduler::submit(ClientIdType clientId, int32_t jobId, uid_t 
    return true;

}



bool TranscodingJobScheduler::cancel(ClientIdType clientId, int32_t jobId) {

bool TranscodingJobScheduler::cancel(ClientIdType clientId, JobIdType jobId) {

    JobKeyType jobKey = std::make_pair(clientId, jobId);



    ALOGV("%s: job %s", __FUNCTION__, jobToString(jobKey).c_str());
@@ -257,7 +257,7 @@ bool TranscodingJobScheduler::cancel(ClientIdType clientId, int32_t jobId) { 
    return true;

}



bool TranscodingJobScheduler::getJob(ClientIdType clientId, int32_t jobId,

bool TranscodingJobScheduler::getJob(ClientIdType clientId, JobIdType jobId,

                                     TranscodingRequestParcel* request) {

    JobKeyType jobKey = std::make_pair(clientId, jobId);
@@ -272,7 +272,7 @@ bool TranscodingJobScheduler::getJob(ClientIdType clientId, int32_t jobId, 
    return true;

}



void TranscodingJobScheduler::onFinish(ClientIdType clientId, int32_t jobId) {

void TranscodingJobScheduler::onFinish(ClientIdType clientId, JobIdType jobId) {

    JobKeyType jobKey = std::make_pair(clientId, jobId);



    ALOGV("%s: job %s", __FUNCTION__, jobToString(jobKey).c_str());
@@ -308,7 +308,8 @@ void TranscodingJobScheduler::onFinish(ClientIdType clientId, int32_t jobId) { 
    validateState_l();

}



void TranscodingJobScheduler::onError(int64_t clientId, int32_t jobId, TranscodingErrorCode err) {

void TranscodingJobScheduler::onError(ClientIdType clientId, JobIdType jobId,

                                      TranscodingErrorCode err) {

    JobKeyType jobKey = std::make_pair(clientId, jobId);



    ALOGV("%s: job %s, err %d", __FUNCTION__, jobToString(jobKey).c_str(), (int32_t)err);
@@ -344,7 +345,8 @@ void TranscodingJobScheduler::onError(int64_t clientId, int32_t jobId, Transcodi 
    validateState_l();

}



void TranscodingJobScheduler::onProgressUpdate(int64_t clientId, int32_t jobId, int32_t progress) {

void TranscodingJobScheduler::onProgressUpdate(ClientIdType clientId, JobIdType jobId,

                                               int32_t progress) {

    JobKeyType jobKey = std::make_pair(clientId, jobId);



    ALOGV("%s: job %s, progress %d", __FUNCTION__, jobToString(jobKey).c_str(), progress);

media/libmediatranscoding/include/media/SchedulerClientInterface.h

+4 −5
Original line number Diff line number Diff line
@@ -19,25 +19,24 @@ 


#include <aidl/android/media/ITranscodingClientCallback.h>

#include <aidl/android/media/TranscodingRequestParcel.h>

#include <media/TranscodingDefs.h>



namespace android {



using ::aidl::android::media::ITranscodingClientCallback;

using ::aidl::android::media::TranscodingRequestParcel;



using ClientIdType = int64_t;



// Interface for a client to call the scheduler to schedule or retrieve

// the status of a job.

class SchedulerClientInterface {

public:

    virtual bool submit(ClientIdType clientId, int32_t jobId, uid_t uid,

    virtual bool submit(ClientIdType clientId, JobIdType jobId, uid_t uid,

                        const TranscodingRequestParcel& request,

                        const std::weak_ptr<ITranscodingClientCallback>& clientCallback) = 0;



    virtual bool cancel(ClientIdType clientId, int32_t jobId) = 0;

    virtual bool cancel(ClientIdType clientId, JobIdType jobId) = 0;



    virtual bool getJob(ClientIdType clientId, int32_t jobId,

    virtual bool getJob(ClientIdType clientId, JobIdType jobId,

                        TranscodingRequestParcel* request) = 0;



protected:

media/libmediatranscoding/include/media/TranscoderInterface.h

+7 −6
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ 
#define ANDROID_MEDIA_TRANSCODER_INTERFACE_H



#include <aidl/android/media/TranscodingErrorCode.h>

#include <media/TranscodingDefs.h>



namespace android {
@@ -30,9 +31,9 @@ public: 
    // TODO(chz): determine what parameters are needed here.

    // For now, always pass in clientId&jobId.

    virtual void setCallback(const std::shared_ptr<TranscoderCallbackInterface>& cb) = 0;

    virtual void start(int64_t clientId, int32_t jobId) = 0;

    virtual void pause(int64_t clientId, int32_t jobId) = 0;

    virtual void resume(int64_t clientId, int32_t jobId) = 0;

    virtual void start(ClientIdType clientId, JobIdType jobId) = 0;

    virtual void pause(ClientIdType clientId, JobIdType jobId) = 0;

    virtual void resume(ClientIdType clientId, JobIdType jobId) = 0;



protected:

    virtual ~TranscoderInterface() = default;
@@ -43,9 +44,9 @@ protected: 
class TranscoderCallbackInterface {

public:

    // TODO(chz): determine what parameters are needed here.

    virtual void onFinish(int64_t clientId, int32_t jobId) = 0;

    virtual void onError(int64_t clientId, int32_t jobId, TranscodingErrorCode err) = 0;

    virtual void onProgressUpdate(int64_t clientId, int32_t jobId, int32_t progress) = 0;

    virtual void onFinish(ClientIdType clientId, JobIdType jobId) = 0;

    virtual void onError(ClientIdType clientId, JobIdType jobId, TranscodingErrorCode err) = 0;

    virtual void onProgressUpdate(ClientIdType clientId, JobIdType jobId, int32_t progress) = 0;



    // Called when transcoding becomes temporarily inaccessible due to loss of resource.

    // If there is any job currently running, it will be paused. When resource contention

media/libmediatranscoding/include/media/TranscodingClientManager.h

+10 −3
Original line number Diff line number Diff line
@@ -24,8 +24,10 @@ 
#include <utils/String8.h>

#include <utils/Vector.h>



#include <map>

#include <mutex>

#include <unordered_map>

#include <unordered_set>



#include "SchedulerClientInterface.h"
@@ -37,9 +39,8 @@ using ::aidl::android::media::ITranscodingClientCallback; 
/*

 * TranscodingClientManager manages all the transcoding clients across different processes.

 *

 * TranscodingClientManager is a global singleton that could only acquired by

 * MediaTranscodingService. It manages all the clients's registration/unregistration and clients'

 * information. It also bookkeeps all the clients' information. It also monitors to the death of the

 * TranscodingClientManager manages all the clients's registration/unregistration and clients'

 * information. It also bookkeeps all the clients' information. It also monitors the death of the

 * clients. Upon client's death, it will remove the client from it.

 *

 * TODO(hkuang): Hook up with ResourceManager for resource management.
@@ -102,10 +103,16 @@ private: 
    mutable std::mutex mLock;

    std::unordered_map<ClientIdType, std::shared_ptr<ClientImpl>> mClientIdToClientMap

            GUARDED_BY(mLock);

    std::unordered_set<uintptr_t> mRegisteredCallbacks GUARDED_BY(mLock);



    ::ndk::ScopedAIBinder_DeathRecipient mDeathRecipient;



    std::shared_ptr<SchedulerClientInterface> mJobScheduler;



    static std::atomic<ClientIdType> sCookieCounter;

    static std::mutex sCookie2ClientLock;

    static std::map<ClientIdType, std::shared_ptr<ClientImpl>> sCookie2Client

            GUARDED_BY(sCookie2ClientLock);

};



}  // namespace android