From 8567cd5af9b7cc6426e9ee89ba240e2a2efa2522 Mon Sep 17 00:00:00 2001 From: Jay Shrauner Date: Wed, 26 Feb 2014 15:33:34 -0800 Subject: Fix threading problems in voicemail code Hold libvariablespeed handle in a reference counted singleton. Don't always reinitialize the engine twice in quick succession on startup. Catch and properly handle RejectedExecutionException exceptions thrown by the executor service. Bug:11511992 Change-Id: I6198dec303209445a0efd0f410a67332d75c7507 --- .../voicemail/VoicemailPlaybackFragment.java | 43 +++++++-- .../voicemail/VoicemailPlaybackPresenter.java | 104 +++++++++++++-------- 2 files changed, 99 insertions(+), 48 deletions(-) (limited to 'src/com/android/dialer/voicemail') diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java b/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java index 1dbae65d1..826dec074 100644 --- a/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java +++ b/src/com/android/dialer/voicemail/VoicemailPlaybackFragment.java @@ -74,7 +74,9 @@ public class VoicemailPlaybackFragment extends Fragment { }; private VoicemailPlaybackPresenter mPresenter; - private ScheduledExecutorService mScheduledExecutorService; + private static int mMediaPlayerRefCount = 0; + private static MediaPlayerProxy mMediaPlayerInstance; + private static ScheduledExecutorService mScheduledExecutorService; private View mPlaybackLayout; @Override @@ -87,7 +89,6 @@ public class VoicemailPlaybackFragment extends Fragment { @Override public void onActivityCreated(Bundle savedInstanceState) { super.onActivityCreated(savedInstanceState); - mScheduledExecutorService = createScheduledExecutorService(); Bundle arguments = getArguments(); Preconditions.checkNotNull(arguments, "fragment must be started with arguments"); Uri voicemailUri = arguments.getParcelable(EXTRA_VOICEMAIL_URI); @@ -99,8 +100,8 @@ public class VoicemailPlaybackFragment extends Fragment { powerManager.newWakeLock( PowerManager.SCREEN_DIM_WAKE_LOCK, getClass().getSimpleName()); mPresenter = new VoicemailPlaybackPresenter(createPlaybackViewImpl(), - createMediaPlayer(mScheduledExecutorService), voicemailUri, - mScheduledExecutorService, startPlayback, + getMediaPlayerInstance(), voicemailUri, + getScheduledExecutorServiceInstance(), startPlayback, AsyncTaskExecutors.createAsyncTaskExecutor(), wakeLock); mPresenter.onCreate(savedInstanceState); } @@ -113,8 +114,8 @@ public class VoicemailPlaybackFragment extends Fragment { @Override public void onDestroy() { + shutdownMediaPlayer(); mPresenter.onDestroy(); - mScheduledExecutorService.shutdown(); super.onDestroy(); } @@ -129,12 +130,36 @@ public class VoicemailPlaybackFragment extends Fragment { mPlaybackLayout); } - private MediaPlayerProxy createMediaPlayer(ExecutorService executorService) { - return VariableSpeed.createVariableSpeed(executorService); + private static synchronized MediaPlayerProxy getMediaPlayerInstance() { + ++mMediaPlayerRefCount; + if (mMediaPlayerInstance == null) { + mMediaPlayerInstance = VariableSpeed.createVariableSpeed( + getScheduledExecutorServiceInstance()); + } + return mMediaPlayerInstance; + } + + private static synchronized ScheduledExecutorService getScheduledExecutorServiceInstance() { + if (mScheduledExecutorService == null) { + mScheduledExecutorService = Executors.newScheduledThreadPool( + NUMBER_OF_THREADS_IN_POOL); + } + return mScheduledExecutorService; } - private ScheduledExecutorService createScheduledExecutorService() { - return Executors.newScheduledThreadPool(NUMBER_OF_THREADS_IN_POOL); + private static synchronized void shutdownMediaPlayer() { + --mMediaPlayerRefCount; + if (mMediaPlayerRefCount > 0) { + return; + } + if (mScheduledExecutorService != null) { + mScheduledExecutorService.shutdown(); + mScheduledExecutorService = null; + } + if (mMediaPlayerInstance != null) { + mMediaPlayerInstance.release(); + mMediaPlayerInstance = null; + } } /** diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java index ebda0eb3c..085ef669a 100644 --- a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java +++ b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java @@ -35,6 +35,7 @@ import com.android.ex.variablespeed.SingleThreadedMediaPlayerProxy; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -311,6 +312,7 @@ public class VoicemailPlaybackPresenter { mPlayer.setDataSource(mView.getDataSourceContext(), mVoicemailUri); mPlayer.setAudioStreamType(PLAYBACK_STREAM); mPlayer.prepare(); + mDuration.set(mPlayer.getDuration()); return null; } catch (Exception e) { return e; @@ -344,7 +346,7 @@ public class VoicemailPlaybackPresenter { mView.setSpeakerPhoneOn(mView.isSpeakerPhoneOn()); mView.setRateDecreaseButtonListener(createRateDecreaseListener()); mView.setRateIncreaseButtonListener(createRateIncreaseListener()); - mView.setClipPosition(0, mPlayer.getDuration()); + mView.setClipPosition(0, mDuration.get()); mView.playbackStopped(); // Always disable on stop. mView.disableProximitySensor(); @@ -363,6 +365,10 @@ public class VoicemailPlaybackPresenter { } public void onDestroy() { + if (mPrepareTask != null) { + mPrepareTask.cancel(false); + mPrepareTask = null; + } mPlayer.release(); if (mFetchResultHandler != null) { mFetchResultHandler.destroy(); @@ -430,49 +436,67 @@ public class VoicemailPlaybackPresenter { } } + private class AsyncPrepareTask extends AsyncTask { + private int mClipPositionInMillis; + + AsyncPrepareTask(int clipPositionInMillis) { + mClipPositionInMillis = clipPositionInMillis; + } + + @Override + public Exception doInBackground(Void... params) { + try { + if (!mPlayer.isReadyToPlay()) { + mPlayer.reset(); + mPlayer.setDataSource(mView.getDataSourceContext(), mVoicemailUri); + mPlayer.setAudioStreamType(PLAYBACK_STREAM); + mPlayer.prepare(); + } + return null; + } catch (Exception e) { + return e; + } + } + + @Override + public void onPostExecute(Exception exception) { + mPrepareTask = null; + if (exception == null) { + final int duration = mPlayer.getDuration(); + mDuration.set(duration); + int startPosition = + constrain(mClipPositionInMillis, 0, duration); + mPlayer.seekTo(startPosition); + mView.setClipPosition(startPosition, duration); + try { + // Can throw RejectedExecutionException + mPlayer.start(); + mView.playbackStarted(); + if (!mWakeLock.isHeld()) { + mWakeLock.acquire(); + } + // Only enable if we are not currently using the speaker phone. + if (!mView.isSpeakerPhoneOn()) { + mView.enableProximitySensor(); + } + // Can throw RejectedExecutionException + mPositionUpdater.startUpdating(startPosition, duration); + } catch (RejectedExecutionException e) { + handleError(e); + } + } else { + handleError(exception); + } + } + } + private void resetPrepareStartPlaying(final int clipPositionInMillis) { if (mPrepareTask != null) { mPrepareTask.cancel(false); + mPrepareTask = null; } mPrepareTask = mAsyncTaskExecutor.submit(Tasks.RESET_PREPARE_START_MEDIA_PLAYER, - new AsyncTask() { - @Override - public Exception doInBackground(Void... params) { - try { - mPlayer.reset(); - mPlayer.setDataSource(mView.getDataSourceContext(), mVoicemailUri); - mPlayer.setAudioStreamType(PLAYBACK_STREAM); - mPlayer.prepare(); - return null; - } catch (Exception e) { - return e; - } - } - - @Override - public void onPostExecute(Exception exception) { - mPrepareTask = null; - if (exception == null) { - mDuration.set(mPlayer.getDuration()); - int startPosition = - constrain(clipPositionInMillis, 0, mDuration.get()); - mView.setClipPosition(startPosition, mDuration.get()); - mPlayer.seekTo(startPosition); - mPlayer.start(); - mView.playbackStarted(); - if (!mWakeLock.isHeld()) { - mWakeLock.acquire(); - } - // Only enable if we are not currently using the speaker phone. - if (!mView.isSpeakerPhoneOn()) { - mView.enableProximitySensor(); - } - mPositionUpdater.startUpdating(startPosition, mDuration.get()); - } else { - handleError(exception); - } - } - }); + new AsyncPrepareTask(clipPositionInMillis)); } private void handleError(Exception e) { @@ -598,6 +622,7 @@ public class VoicemailPlaybackPresenter { synchronized (mLock) { if (mScheduledFuture != null) { mScheduledFuture.cancel(false); + mScheduledFuture = null; } mScheduledFuture = mExecutorService.scheduleAtFixedRate(this, 0, mPeriodMillis, TimeUnit.MILLISECONDS); @@ -620,6 +645,7 @@ public class VoicemailPlaybackPresenter { } if (mPrepareTask != null) { mPrepareTask.cancel(false); + mPrepareTask = null; } if (mWakeLock.isHeld()) { mWakeLock.release(); -- cgit v1.2.3