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

Commit 02b7421d authored by Chris Thornton's avatar Chris Thornton
Browse files

Fix race when handling callback in SoundTrigger Module

During the callback event processing loop in the SoundTrigger::Module,
it used to grab the ISoundTriggerClient from the ModuleClient that it
needed to send an event to. However, this could be removed/destroyed
while the callback thread is trying to snapshot it, since there was no
common locking between the model client and the module.

By now using the ModuleClient directly in the Module's callback, the
appropriate locks can be obtained to prevent segfaults.

Test: On device with GSA/Now Playing
Change-Id: I9e3d150aa77051caab1fc009c3fc1ae8eec61a72
parent 8f8932c4
Loading
Loading
Loading
Loading
+27 −26
Original line number Original line Diff line number Diff line
@@ -750,11 +750,12 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve
        return;
        return;
    }
    }


    Vector< sp<ModuleClient> > clients;

    switch (event->mType) {
    switch (event->mType) {
    case CallbackEvent::TYPE_RECOGNITION: {
    case CallbackEvent::TYPE_RECOGNITION: {
        struct sound_trigger_recognition_event *recognitionEvent =
        struct sound_trigger_recognition_event *recognitionEvent =
                (struct sound_trigger_recognition_event *)eventMemory->pointer();
                (struct sound_trigger_recognition_event *)eventMemory->pointer();
        sp<ISoundTriggerClient> client;
        {
        {
            AutoMutex lock(mLock);
            AutoMutex lock(mLock);
            sp<Model> model = getModel(recognitionEvent->model);
            sp<Model> model = getModel(recognitionEvent->model);
@@ -769,16 +770,12 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve


            recognitionEvent->capture_session = model->mCaptureSession;
            recognitionEvent->capture_session = model->mCaptureSession;
            model->mState = Model::STATE_IDLE;
            model->mState = Model::STATE_IDLE;
            client = model->mModuleClient->client();
            clients.add(model->mModuleClient);
        }
        if (client != 0) {
            client->onRecognitionEvent(eventMemory);
        }
        }
    } break;
    } break;
    case CallbackEvent::TYPE_SOUNDMODEL: {
    case CallbackEvent::TYPE_SOUNDMODEL: {
        struct sound_trigger_model_event *soundmodelEvent =
        struct sound_trigger_model_event *soundmodelEvent =
                (struct sound_trigger_model_event *)eventMemory->pointer();
                (struct sound_trigger_model_event *)eventMemory->pointer();
        sp<ISoundTriggerClient> client;
        {
        {
            AutoMutex lock(mLock);
            AutoMutex lock(mLock);
            sp<Model> model = getModel(soundmodelEvent->model);
            sp<Model> model = getModel(soundmodelEvent->model);
@@ -786,29 +783,26 @@ void SoundTriggerHwService::Module::onCallbackEvent(const sp<CallbackEvent>& eve
                ALOGW("%s model == 0", __func__);
                ALOGW("%s model == 0", __func__);
                return;
                return;
            }
            }
            client = model->mModuleClient->client();
            clients.add(model->mModuleClient);
        }
        if (client != 0) {
            client->onSoundModelEvent(eventMemory);
        }
        }
    } break;
    } break;
    case CallbackEvent::TYPE_SERVICE_STATE: {
    case CallbackEvent::TYPE_SERVICE_STATE: {
        Vector< sp<ISoundTriggerClient> > clients;
        {
        {
            AutoMutex lock(mLock);
            AutoMutex lock(mLock);
            for (size_t i = 0; i < mModuleClients.size(); i++) {
            for (size_t i = 0; i < mModuleClients.size(); i++) {
                if (mModuleClients[i] != 0) {
                if (mModuleClients[i] != 0) {
                    clients.add(mModuleClients[i]->client());
                    clients.add(mModuleClients[i]);
                }
                }
            }
            }
        }
        }
        for (size_t i = 0; i < clients.size(); i++) {
            clients[i]->onServiceStateChange(eventMemory);
        }
    } break;
    } break;
    default:
    default:
        LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
        LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
    }
    }

    for (size_t i = 0; i < clients.size(); i++) {
        clients[i]->onCallbackEvent(event);
    }
}
}


sp<SoundTriggerHwService::Model> SoundTriggerHwService::Module::getModel(
sp<SoundTriggerHwService::Model> SoundTriggerHwService::Module::getModel(
@@ -1070,21 +1064,28 @@ void SoundTriggerHwService::ModuleClient::onCallbackEvent(const sp<CallbackEvent
        return;
        return;
    }
    }


    switch (event->mType) {
    case CallbackEvent::TYPE_SERVICE_STATE: {
    sp<ISoundTriggerClient> client;
    sp<ISoundTriggerClient> client;
    {
    {
        AutoMutex lock(mLock);
        AutoMutex lock(mLock);
        client = mClient;
        client = mClient;
    }
    }

    if (client != 0) {
    if (client != 0) {
        switch (event->mType) {
        case CallbackEvent::TYPE_RECOGNITION: {
            client->onRecognitionEvent(eventMemory);
        } break;
        case CallbackEvent::TYPE_SOUNDMODEL: {
            client->onSoundModelEvent(eventMemory);
        } break;
        case CallbackEvent::TYPE_SERVICE_STATE: {
            client->onServiceStateChange(eventMemory);
            client->onServiceStateChange(eventMemory);
        }
        } break;
        } break;
        default:
        default:
            LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
            LOG_ALWAYS_FATAL("onCallbackEvent unknown event type %d", event->mType);
        }
        }
    }
    }
}


void SoundTriggerHwService::ModuleClient::binderDied(
void SoundTriggerHwService::ModuleClient::binderDied(
    const wp<IBinder> &who __unused) {
    const wp<IBinder> &who __unused) {