Merge "codec2: BufferPoolClient: fix NPE in fetchBufferHandle if connection is null" into main
diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp
index 389860f..eecc972 100644
--- a/audio/aidl/default/Stream.cpp
+++ b/audio/aidl/default/Stream.cpp
@@ -663,10 +663,14 @@
}
StreamCommonImpl::~StreamCommonImpl() {
- if (!isClosed()) {
- LOG(ERROR) << __func__ << ": stream was not closed prior to destruction, resource leak";
- stopWorker();
- // The worker and the context should clean up by themselves via destructors.
+ // It is responsibility of the class that implements 'DriverInterface' to call 'cleanupWorker'
+ // in the destructor. Note that 'cleanupWorker' can not be properly called from this destructor
+ // because any subclasses have already been destroyed and thus the 'DriverInterface'
+ // implementation is not valid. Thus, here it can only be asserted whether the subclass has done
+ // its job.
+ if (!mWorkerStopIssued && !isClosed()) {
+ LOG(FATAL) << __func__ << ": the stream implementation must call 'cleanupWorker' "
+ << "in order to clean up the worker thread.";
}
}
@@ -770,10 +774,7 @@
ndk::ScopedAStatus StreamCommonImpl::close() {
LOG(DEBUG) << __func__;
if (!isClosed()) {
- stopWorker();
- LOG(DEBUG) << __func__ << ": joining the worker thread...";
- mWorker->join();
- LOG(DEBUG) << __func__ << ": worker thread joined";
+ stopAndJoinWorker();
onClose(mWorker->setClosed());
return ndk::ScopedAStatus::ok();
} else {
@@ -791,6 +792,20 @@
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
}
+void StreamCommonImpl::cleanupWorker() {
+ if (!isClosed()) {
+ LOG(ERROR) << __func__ << ": stream was not closed prior to destruction, resource leak";
+ stopAndJoinWorker();
+ }
+}
+
+void StreamCommonImpl::stopAndJoinWorker() {
+ stopWorker();
+ LOG(DEBUG) << __func__ << ": joining the worker thread...";
+ mWorker->join();
+ LOG(DEBUG) << __func__ << ": worker thread joined";
+}
+
void StreamCommonImpl::stopWorker() {
if (auto commandMQ = mContext.getCommandMQ(); commandMQ != nullptr) {
LOG(DEBUG) << __func__ << ": asking the worker to exit...";
@@ -805,6 +820,7 @@
}
LOG(DEBUG) << __func__ << ": done";
}
+ mWorkerStopIssued = true;
}
ndk::ScopedAStatus StreamCommonImpl::updateMetadataCommon(const Metadata& metadata) {
diff --git a/audio/aidl/default/alsa/StreamAlsa.cpp b/audio/aidl/default/alsa/StreamAlsa.cpp
index e57d538..f548903 100644
--- a/audio/aidl/default/alsa/StreamAlsa.cpp
+++ b/audio/aidl/default/alsa/StreamAlsa.cpp
@@ -37,6 +37,10 @@
mConfig(alsa::getPcmConfig(getContext(), mIsInput)),
mReadWriteRetries(readWriteRetries) {}
+StreamAlsa::~StreamAlsa() {
+ cleanupWorker();
+}
+
::android::status_t StreamAlsa::init() {
return mConfig.has_value() ? ::android::OK : ::android::NO_INIT;
}
diff --git a/audio/aidl/default/bluetooth/StreamBluetooth.cpp b/audio/aidl/default/bluetooth/StreamBluetooth.cpp
index efab470..6e1a811 100644
--- a/audio/aidl/default/bluetooth/StreamBluetooth.cpp
+++ b/audio/aidl/default/bluetooth/StreamBluetooth.cpp
@@ -66,6 +66,10 @@
1000),
mBtDeviceProxy(btDeviceProxy) {}
+StreamBluetooth::~StreamBluetooth() {
+ cleanupWorker();
+}
+
::android::status_t StreamBluetooth::init() {
std::lock_guard guard(mLock);
if (mBtDeviceProxy == nullptr) {
diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h
index 93ace96..100b4c8 100644
--- a/audio/aidl/default/include/core-impl/Stream.h
+++ b/audio/aidl/default/include/core-impl/Stream.h
@@ -457,6 +457,11 @@
}
virtual void onClose(StreamDescriptor::State statePriorToClosing) = 0;
+ // Any stream class implementing 'DriverInterface::shutdown' must call 'cleanupWorker' in
+ // the destructor in order to stop and join the worker thread in the case when the client
+ // has not called 'IStreamCommon::close' method.
+ void cleanupWorker();
+ void stopAndJoinWorker();
void stopWorker();
const StreamContext& mContext;
@@ -464,6 +469,9 @@
std::unique_ptr<StreamWorkerInterface> mWorker;
ChildInterface<StreamCommonDelegator> mCommon;
ConnectedDevices mConnectedDevices;
+
+ private:
+ std::atomic<bool> mWorkerStopIssued = false;
};
// Note: 'StreamIn/Out' can not be used on their own. Instead, they must be used for defining
diff --git a/audio/aidl/default/include/core-impl/StreamAlsa.h b/audio/aidl/default/include/core-impl/StreamAlsa.h
index 2c3b284..0356946 100644
--- a/audio/aidl/default/include/core-impl/StreamAlsa.h
+++ b/audio/aidl/default/include/core-impl/StreamAlsa.h
@@ -32,6 +32,8 @@
class StreamAlsa : public StreamCommonImpl {
public:
StreamAlsa(StreamContext* context, const Metadata& metadata, int readWriteRetries);
+ ~StreamAlsa();
+
// Methods of 'DriverInterface'.
::android::status_t init() override;
::android::status_t drain(StreamDescriptor::DrainMode) override;
diff --git a/audio/aidl/default/include/core-impl/StreamBluetooth.h b/audio/aidl/default/include/core-impl/StreamBluetooth.h
index 7f4239c..357a546 100644
--- a/audio/aidl/default/include/core-impl/StreamBluetooth.h
+++ b/audio/aidl/default/include/core-impl/StreamBluetooth.h
@@ -41,6 +41,8 @@
const std::shared_ptr<::android::bluetooth::audio::aidl::BluetoothAudioPortAidl>&
btDeviceProxy,
const ::aidl::android::hardware::bluetooth::audio::PcmConfiguration& pcmConfig);
+ ~StreamBluetooth();
+
// Methods of 'DriverInterface'.
::android::status_t init() override;
::android::status_t drain(StreamDescriptor::DrainMode) override;
diff --git a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h
index 0d50c96..6ea7968 100644
--- a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h
+++ b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h
@@ -29,7 +29,9 @@
StreamRemoteSubmix(
StreamContext* context, const Metadata& metadata,
const ::aidl::android::media::audio::common::AudioDeviceAddress& deviceAddress);
+ ~StreamRemoteSubmix();
+ // Methods of 'DriverInterface'.
::android::status_t init() override;
::android::status_t drain(StreamDescriptor::DrainMode) override;
::android::status_t flush() override;
diff --git a/audio/aidl/default/include/core-impl/StreamStub.h b/audio/aidl/default/include/core-impl/StreamStub.h
index 3857e0e..f4ee566 100644
--- a/audio/aidl/default/include/core-impl/StreamStub.h
+++ b/audio/aidl/default/include/core-impl/StreamStub.h
@@ -23,6 +23,8 @@
class StreamStub : public StreamCommonImpl {
public:
StreamStub(StreamContext* context, const Metadata& metadata);
+ ~StreamStub();
+
// Methods of 'DriverInterface'.
::android::status_t init() override;
::android::status_t drain(StreamDescriptor::DrainMode) override;
diff --git a/audio/aidl/default/include/core-impl/StreamUsb.h b/audio/aidl/default/include/core-impl/StreamUsb.h
index 608f27d..694fccf 100644
--- a/audio/aidl/default/include/core-impl/StreamUsb.h
+++ b/audio/aidl/default/include/core-impl/StreamUsb.h
@@ -29,6 +29,7 @@
class StreamUsb : public StreamAlsa {
public:
StreamUsb(StreamContext* context, const Metadata& metadata);
+
// Methods of 'DriverInterface'.
::android::status_t transfer(void* buffer, size_t frameCount, size_t* actualFrameCount,
int32_t* latencyMs) override;
diff --git a/audio/aidl/default/main.cpp b/audio/aidl/default/main.cpp
index 6ab747d..0b3e3ba 100644
--- a/audio/aidl/default/main.cpp
+++ b/audio/aidl/default/main.cpp
@@ -71,6 +71,7 @@
// For more logs, use VERBOSE, however this may hinder performance.
// android::base::SetMinimumLogSeverity(::android::base::VERBOSE);
ABinderProcess_setThreadPoolMaxThreadCount(16);
+ ABinderProcess_startThreadPool();
// Guaranteed log for b/210919187 and logd_integration_test
LOG(INFO) << "Init for Audio AIDL HAL";
diff --git a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
index a266b54..db105b6 100644
--- a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
+++ b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
@@ -43,6 +43,10 @@
mStreamConfig.sampleRate = context->getSampleRate();
}
+StreamRemoteSubmix::~StreamRemoteSubmix() {
+ cleanupWorker();
+}
+
::android::status_t StreamRemoteSubmix::init() {
mCurrentRoute = SubmixRoute::findOrCreateRoute(mDeviceAddress, mStreamConfig);
if (mCurrentRoute == nullptr) {
diff --git a/audio/aidl/default/stub/StreamStub.cpp b/audio/aidl/default/stub/StreamStub.cpp
index 2422fe4..3b6a85a 100644
--- a/audio/aidl/default/stub/StreamStub.cpp
+++ b/audio/aidl/default/stub/StreamStub.cpp
@@ -39,6 +39,10 @@
mIsAsynchronous(!!getContext().getAsyncCallback()),
mIsInput(isInput(metadata)) {}
+StreamStub::~StreamStub() {
+ cleanupWorker();
+}
+
::android::status_t StreamStub::init() {
mIsInitialized = true;
return ::android::OK;