MediaPlayerService: fix access of mPlayer in client
Test: poc doesn't crash
Bug: 38234812
Change-Id: I6f9be046ff66d2d5bed27bd712287e4ead550830
diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index c3dbc1b..cf59717 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -79,6 +79,9 @@
#include "HTTPBase.h"
#include "RemoteDisplay.h"
+static const int kDumpLockRetries = 50;
+static const int kDumpLockSleepUs = 20000;
+
namespace {
using android::media::Metadata;
using android::status_t;
@@ -366,12 +369,32 @@
snprintf(buffer, 255, " pid(%d), connId(%d), status(%d), looping(%s)\n",
mPid, mConnId, mStatus, mLoop?"true": "false");
result.append(buffer);
- write(fd, result.string(), result.size());
- if (mPlayer != NULL) {
- mPlayer->dump(fd, args);
+
+ sp<MediaPlayerBase> p;
+ sp<AudioOutput> audioOutput;
+ bool locked = false;
+ for (int i = 0; i < kDumpLockRetries; ++i) {
+ if (mLock.tryLock() == NO_ERROR) {
+ locked = true;
+ break;
+ }
+ usleep(kDumpLockSleepUs);
}
- if (mAudioOutput != 0) {
- mAudioOutput->dump(fd, args);
+
+ if (locked) {
+ p = mPlayer;
+ audioOutput = mAudioOutput;
+ mLock.unlock();
+ } else {
+ result.append(" lock is taken, no dump from player and audio output\n");
+ }
+ write(fd, result.string(), result.size());
+
+ if (p != NULL) {
+ p->dump(fd, args);
+ }
+ if (audioOutput != 0) {
+ audioOutput->dump(fd, args);
}
write(fd, "\n", 1);
return NO_ERROR;
@@ -528,7 +551,10 @@
MediaPlayerService::Client::~Client()
{
ALOGV("Client(%d) destructor pid = %d", mConnId, mPid);
- mAudioOutput.clear();
+ {
+ Mutex::Autolock l(mLock);
+ mAudioOutput.clear();
+ }
wp<Client> client(this);
disconnect();
mService->removeClient(client);
@@ -544,10 +570,9 @@
Mutex::Autolock l(mLock);
p = mPlayer;
mClient.clear();
+ mPlayer.clear();
}
- mPlayer.clear();
-
// clear the notification to prevent callbacks to dead client
// and reset the player. We assume the player will serialize
// access to itself if necessary.
@@ -568,7 +593,7 @@
sp<MediaPlayerBase> MediaPlayerService::Client::createPlayer(player_type playerType)
{
// determine if we have the right player type
- sp<MediaPlayerBase> p = mPlayer;
+ sp<MediaPlayerBase> p = getPlayer();
if ((p != NULL) && (p->playerType() != playerType)) {
ALOGV("delete player");
p.clear();
@@ -623,6 +648,7 @@
}
if (mStatus == OK) {
+ Mutex::Autolock l(mLock);
mPlayer = p;
}
}