From 7d6d9bd6c3693563ffe14a32d5aac08df69bb04c Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 8 Sep 2021 17:58:53 -0700 Subject: [PATCH] Analyze native crash in AudioEffect via event listener Add repro code to the EffectsTest app. Steps: 1. Start playback on the "Environmental Reverb Test", memorize the session id. 2. Enter the session id on the "Bass Boost Test" or the "Visualizer" tab. 3. Press "Hammer on release()" multiple times and check if the app crashes. Currently this happens for the Bass Boost effect. Bug: 178363662 Test: see above Change-Id: I800bc352fc8bc7073d1523c837008dc6fcfb957a --- media/tests/EffectsTest/AndroidManifest.xml | 4 + .../EffectsTest/res/layout/bassboosttest.xml | 5 ++ .../EffectsTest/res/layout/visualizertest.xml | 5 ++ .../tests/EffectsTest/res/values/strings.xml | 2 + .../android/effectstest/BassBoostTest.java | 84 +++++++++++++++---- .../android/effectstest/VisualizerTest.java | 60 +++++++++++++ 6 files changed, 145 insertions(+), 15 deletions(-) diff --git a/media/tests/EffectsTest/AndroidManifest.xml b/media/tests/EffectsTest/AndroidManifest.xml index 9b59891fb2796..ad0c10ee38072 100644 --- a/media/tests/EffectsTest/AndroidManifest.xml +++ b/media/tests/EffectsTest/AndroidManifest.xml @@ -13,6 +13,10 @@ See the License for the specific language governing permissions and limitations under the License. --> + diff --git a/media/tests/EffectsTest/res/layout/bassboosttest.xml b/media/tests/EffectsTest/res/layout/bassboosttest.xml index ac912c84d107b..5f9132cb8cc77 100644 --- a/media/tests/EffectsTest/res/layout/bassboosttest.xml +++ b/media/tests/EffectsTest/res/layout/bassboosttest.xml @@ -187,6 +187,11 @@ android:layout_height="wrap_content" android:scaleType="fitXY"/> + + diff --git a/media/tests/EffectsTest/res/layout/visualizertest.xml b/media/tests/EffectsTest/res/layout/visualizertest.xml index 18d7a3648fbfe..85dabbc115f3f 100644 --- a/media/tests/EffectsTest/res/layout/visualizertest.xml +++ b/media/tests/EffectsTest/res/layout/visualizertest.xml @@ -175,6 +175,11 @@ + + Send Level Multithreaded Use + + Hammer on release() diff --git a/media/tests/EffectsTest/src/com/android/effectstest/BassBoostTest.java b/media/tests/EffectsTest/src/com/android/effectstest/BassBoostTest.java index cce2acc5869a8..a207bf1d53597 100644 --- a/media/tests/EffectsTest/src/com/android/effectstest/BassBoostTest.java +++ b/media/tests/EffectsTest/src/com/android/effectstest/BassBoostTest.java @@ -17,29 +17,24 @@ package com.android.effectstest; import android.app.Activity; -import android.content.Context; -import android.content.Intent; +import android.media.audiofx.AudioEffect; +import android.media.audiofx.BassBoost; import android.os.Bundle; import android.util.Log; import android.view.KeyEvent; -import android.view.Menu; -import android.view.View.OnClickListener; import android.view.View; -import android.view.ViewGroup; +import android.view.View.OnClickListener; import android.widget.Button; -import android.widget.TextView; -import android.widget.EditText; -import android.widget.SeekBar; -import android.widget.ToggleButton; import android.widget.CompoundButton; import android.widget.CompoundButton.OnCheckedChangeListener; -import java.nio.ByteOrder; -import java.nio.ByteBuffer; -import java.util.HashMap; -import java.util.Map; +import android.widget.EditText; +import android.widget.SeekBar; +import android.widget.TextView; +import android.widget.ToggleButton; -import android.media.audiofx.BassBoost; -import android.media.audiofx.AudioEffect; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.util.HashMap; public class BassBoostTest extends Activity implements OnCheckedChangeListener { @@ -78,6 +73,9 @@ public class BassBoostTest extends Activity implements OnCheckedChangeListener { mReleaseButton = (ToggleButton)findViewById(R.id.bbReleaseButton); mOnOffButton = (ToggleButton)findViewById(R.id.bassboostOnOff); + final Button hammerReleaseTest = (Button) findViewById(R.id.hammer_on_release_bug); + hammerReleaseTest.setEnabled(false); + getEffect(sSession); if (mBassBoost != null) { @@ -93,6 +91,14 @@ public class BassBoostTest extends Activity implements OnCheckedChangeListener { mStrength = new BassBoostParam(mBassBoost, 0, 1000, seekBar, textView); seekBar.setOnSeekBarChangeListener(mStrength); mStrength.setEnabled(mBassBoost.getStrengthSupported()); + + hammerReleaseTest.setEnabled(true); + hammerReleaseTest.setOnClickListener(new OnClickListener() { + @Override + public void onClick(View v) { + runHammerReleaseTest(hammerReleaseTest); + } + }); } } @@ -273,4 +279,52 @@ public class BassBoostTest extends Activity implements OnCheckedChangeListener { } } + // Stress-tests releasing of AudioEffect by doing repeated creation + // and subsequent releasing. Also forces emission of callbacks from + // the AudioFlinger by setting a control status listener. Since all + // effect instances are bound to the same session, the AF will + // notify them about the change in their status. This can reveal racy + // behavior w.r.t. releasing. + class HammerReleaseTest extends Thread { + private static final int NUM_EFFECTS = 10; + private static final int NUM_ITERATIONS = 100; + private final int mSession; + private final Runnable mOnComplete; + + HammerReleaseTest(int session, Runnable onComplete) { + mSession = session; + mOnComplete = onComplete; + } + + @Override + public void run() { + Log.w(TAG, "HammerReleaseTest started"); + BassBoost[] effects = new BassBoost[NUM_EFFECTS]; + for (int i = 0; i < NUM_ITERATIONS; i++) { + for (int j = 0; j < NUM_EFFECTS; j++) { + effects[j] = new BassBoost(0, mSession); + effects[j].setControlStatusListener(mEffectListener); + yield(); + } + for (int j = NUM_EFFECTS - 1; j >= 0; j--) { + Log.w(TAG, "HammerReleaseTest releasing effect " + (Object) effects[j]); + effects[j].release(); + effects[j] = null; + yield(); + } + } + Log.w(TAG, "HammerReleaseTest ended"); + runOnUiThread(mOnComplete); + } + } + + private void runHammerReleaseTest(Button controlButton) { + controlButton.setEnabled(false); + HammerReleaseTest thread = new HammerReleaseTest(sSession, + () -> { + controlButton.setEnabled(true); + }); + thread.start(); + } + } diff --git a/media/tests/EffectsTest/src/com/android/effectstest/VisualizerTest.java b/media/tests/EffectsTest/src/com/android/effectstest/VisualizerTest.java index 2e141c5ef7c8a..dcfe11a79ef92 100644 --- a/media/tests/EffectsTest/src/com/android/effectstest/VisualizerTest.java +++ b/media/tests/EffectsTest/src/com/android/effectstest/VisualizerTest.java @@ -17,6 +17,7 @@ package com.android.effectstest; import android.app.Activity; +import android.media.audiofx.Visualizer; import android.os.Bundle; import android.os.Handler; import android.os.Looper; @@ -24,6 +25,8 @@ import android.os.Message; import android.util.Log; import android.view.KeyEvent; import android.view.View; +import android.view.View.OnClickListener; +import android.widget.Button; import android.widget.CompoundButton; import android.widget.CompoundButton.OnCheckedChangeListener; import android.widget.EditText; @@ -74,11 +77,22 @@ public class VisualizerTest extends Activity implements OnCheckedChangeListener mCallbackOn = false; mCallbackButton.setChecked(mCallbackOn); + final Button hammerReleaseTest = (Button) findViewById(R.id.hammer_on_release_bug); + hammerReleaseTest.setEnabled(false); + mMultithreadedButton.setOnCheckedChangeListener(this); if (getEffect(sSession) != null) { mReleaseButton.setOnCheckedChangeListener(this); mOnOffButton.setOnCheckedChangeListener(this); mCallbackButton.setOnCheckedChangeListener(this); + + hammerReleaseTest.setEnabled(true); + hammerReleaseTest.setOnClickListener(new OnClickListener() { + @Override + public void onClick(View v) { + runHammerReleaseTest(hammerReleaseTest); + } + }); } } @@ -214,4 +228,50 @@ public class VisualizerTest extends Activity implements OnCheckedChangeListener } } + // Stress-tests releasing of AudioEffect by doing repeated creation + // and subsequent releasing. Unlike a similar class in BassBoostTest, + // this one doesn't sets a control status listener because Visualizer + // doesn't inherit from AudioEffect and doesn't implement this method + // by itself. + class HammerReleaseTest extends Thread { + private static final int NUM_EFFECTS = 10; + private static final int NUM_ITERATIONS = 100; + private final int mSession; + private final Runnable mOnComplete; + + HammerReleaseTest(int session, Runnable onComplete) { + mSession = session; + mOnComplete = onComplete; + } + + @Override + public void run() { + Log.w(TAG, "HammerReleaseTest started"); + Visualizer[] effects = new Visualizer[NUM_EFFECTS]; + for (int i = 0; i < NUM_ITERATIONS; i++) { + for (int j = 0; j < NUM_EFFECTS; j++) { + effects[j] = new Visualizer(mSession); + yield(); + } + for (int j = NUM_EFFECTS - 1; j >= 0; j--) { + Log.w(TAG, "HammerReleaseTest releasing effect " + (Object) effects[j]); + effects[j].release(); + effects[j] = null; + yield(); + } + } + Log.w(TAG, "HammerReleaseTest ended"); + runOnUiThread(mOnComplete); + } + } + + private void runHammerReleaseTest(Button controlButton) { + controlButton.setEnabled(false); + HammerReleaseTest thread = new HammerReleaseTest(sSession, + () -> { + controlButton.setEnabled(true); + }); + thread.start(); + } + }