Commit b1701610 authored by Ken Rockot's avatar Ken Rockot Committed by Commit Bot

Fix threading issue in Assistant Service

With the recent refactor off of Service Manager, a bug was introduced
which can cause the Client interface to be used from arbitrary threads
via calls from libassistant.

This fixes the bug by ensuring that calls to RequestAudioStreamFactory
always happen from the main thread.

Bug: b:139480195
Change-Id: Ie3dca871929ccc913d7d4f1bf3ad2e17b8db7a3d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1756546
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687370}
parent 8c1592af
......@@ -166,8 +166,11 @@ assistant_client::AudioOutput* AudioOutputProviderImpl::CreateAudioOutput(
assistant_client::OutputStreamType type,
const assistant_client::OutputStreamFormat& stream_format) {
mojo::PendingRemote<audio::mojom::StreamFactory> stream_factory;
client_->RequestAudioStreamFactory(
stream_factory.InitWithNewPipeAndPassReceiver());
main_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&AudioOutputProviderImpl::BindStreamFactory,
weak_ptr_factory_.GetWeakPtr(),
stream_factory.InitWithNewPipeAndPassReceiver()));
// Owned by one arbitrary thread inside libassistant. It will be destroyed
// once assistant_client::AudioOutput::Delegate::OnStopped() is called.
return new AudioOutputImpl(std::move(stream_factory), main_task_runner_,
......@@ -204,5 +207,10 @@ void AudioOutputProviderImpl::RegisterAudioEmittingStateCallback(
// TODO(muyuanli): implement.
}
void AudioOutputProviderImpl::BindStreamFactory(
mojo::PendingReceiver<audio::mojom::StreamFactory> receiver) {
client_->RequestAudioStreamFactory(std::move(receiver));
}
} // namespace assistant
} // namespace chromeos
......@@ -11,6 +11,7 @@
#include "base/component_export.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/single_thread_task_runner.h"
#include "chromeos/services/assistant/platform/audio_device_owner.h"
#include "chromeos/services/assistant/platform/audio_input_impl.h"
......@@ -19,6 +20,8 @@
#include "chromeos/services/assistant/public/mojom/assistant_audio_decoder.mojom.h"
#include "libassistant/shared/public/platform_audio_output.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "services/audio/public/mojom/stream_factory.mojom.h"
namespace chromeos {
namespace assistant {
......@@ -52,6 +55,9 @@ class AudioOutputProviderImpl : public assistant_client::AudioOutputProvider {
AudioEmittingStateCallback callback) override;
private:
void BindStreamFactory(
mojo::PendingReceiver<audio::mojom::StreamFactory> receiver);
mojom::Client* const client_;
AudioInputImpl loop_back_input_;
VolumeControlImpl volume_control_impl_;
......@@ -61,6 +67,7 @@ class AudioOutputProviderImpl : public assistant_client::AudioOutputProvider {
mojom::AssistantAudioDecoderFactory* audio_decoder_factory_;
std::string device_id_;
AssistantMediaSession* media_session_;
base::WeakPtrFactory<AudioOutputProviderImpl> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(AudioOutputProviderImpl);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment