commit bd6dc464586edade39b3bcac6f6f0d3b2d5d1077
author zengjing <zengjing@xiaomi.com> Sat Sep 29 13:25:35 2018 +0800
committer Mikhail Naganov <mnaganov@google.com> Fri Feb 22 19:28:36 2019 +0000
tree e44372632e4aa74d082a2e324a9585902a726ecd
parent a504d888d4cc53d8067bb67f9d7f1c063ca54a93

Visualizer: fix native crash when visualizer release

When the Visualizer effect is released, synchronous wait for the CaptureThread can
cause ANR in an app. This is why an asynchronous 'release' method is introduced which
is used by Visualizer.release() method on the Java side. Since CaptureThread may still
be running while the the last reference to the Visualizer instance is released,
CaptureThread now holds a strong reference to the Visualizer, which it releases
upon exit from the thread loop.

The 'release' method does not check for 'enabled' status because setEnabled(false)
may fail due to audioserver calling AudioFlinger::EffectHandle::setControl to
hold control.

Bug: 124833084
Test: CTS VisualizerTest
Change-Id: I8d936f0f79540345d3e3675f6129bb942a08e423
Merged-In: I8d936f0f79540345d3e3675f6129bb942a08e423
Signed-off-by: zengjing <zengjing@xiaomi.com>

media/libmedia/Visualizer.cpp

diff --git a/media/libmedia/Visualizer.cpp b/media/libmedia/Visualizer.cpp
index 4984b18..cb8d375 100644
--- a/media/libmedia/Visualizer.cpp
+++ b/media/libmedia/Visualizer.cpp
@@ -56,6 +56,19 @@
     setCaptureCallBack(NULL, NULL, 0, 0);
 }
 
+void Visualizer::release()
+{
+    ALOGV("Visualizer::release()");
+    setEnabled(false);
+    Mutex::Autolock _l(mCaptureLock);
+
+    mCaptureThread.clear();
+    mCaptureCallBack = NULL;
+    mCaptureCbkUser = NULL;
+    mCaptureFlags = 0;
+    mCaptureRate = 0;
+}
+
 status_t Visualizer::setEnabled(bool enabled)
 {
     Mutex::Autolock _l(mCaptureLock);
@@ -115,7 +128,7 @@
     mCaptureRate = rate;
 
     if (cbk != NULL) {
-        mCaptureThread = new CaptureThread(*this, rate, ((flags & CAPTURE_CALL_JAVA) != 0));
+        mCaptureThread = new CaptureThread(this, rate, ((flags & CAPTURE_CALL_JAVA) != 0));
     }
     ALOGV("setCaptureCallBack() rate: %d thread %p flags 0x%08x",
             rate, mCaptureThread.get(), mCaptureFlags);
@@ -402,7 +415,7 @@
 
 //-------------------------------------------------------------------------
 
-Visualizer::CaptureThread::CaptureThread(Visualizer& receiver, uint32_t captureRate,
+Visualizer::CaptureThread::CaptureThread(Visualizer* receiver, uint32_t captureRate,
         bool bCanCallJava)
     : Thread(bCanCallJava), mReceiver(receiver)
 {
@@ -413,10 +426,14 @@
 bool Visualizer::CaptureThread::threadLoop()
 {
     ALOGV("CaptureThread %p enter", this);
+    sp<Visualizer> receiver = mReceiver.promote();
+    if (receiver == NULL) {
+        return false;
+    }
     while (!exitPending())
     {
         usleep(mSleepTimeUs);
-        mReceiver.periodicCapture();
+        receiver->periodicCapture();
     }
     ALOGV("CaptureThread %p exiting", this);
     return false;

media/libmedia/include/media/Visualizer.h

diff --git a/media/libmedia/include/media/Visualizer.h b/media/libmedia/include/media/Visualizer.h
index f8f4f50..8078e36 100644
--- a/media/libmedia/include/media/Visualizer.h
+++ b/media/libmedia/include/media/Visualizer.h
@@ -131,6 +131,7 @@
     // getCaptureSize() but the length of the FFT is half of the size (both parts of the spectrum
     // are returned
     status_t getFft(uint8_t *fft);
+    void release();
 
 protected:
     // from IEffectClient
@@ -146,12 +147,12 @@
     class CaptureThread : public Thread
     {
     public:
-        CaptureThread(Visualizer& receiver, uint32_t captureRate, bool bCanCallJava = false);
+        CaptureThread(Visualizer* visualizer, uint32_t captureRate, bool bCanCallJava = false);
 
     private:
         friend class Visualizer;
         virtual bool        threadLoop();
-        Visualizer& mReceiver;
+        wp<Visualizer> mReceiver;
         Mutex       mLock;
         uint32_t mSleepTimeUs;
     };