Skip to content

Commit df382ca

Browse files
committed
Be more paranoid about threading in MediaSessionRecord
This makes copies of objects with bundles before posting to another thread and is more aggressive about locking before making assignments of mutable objects. bug:17692568 Change-Id: I28e8229718b862c485e870fd2ca06a3a18a7c454
1 parent 485f209 commit df382ca

1 file changed

Lines changed: 29 additions & 9 deletions

File tree

services/core/java/com/android/server/media/MediaSessionRecord.java

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,10 @@ private void pushSessionDestroyed() {
615615
}
616616

617617
private PlaybackState getStateWithUpdatedPosition() {
618-
PlaybackState state = mPlaybackState;
618+
PlaybackState state;
619+
synchronized (mLock) {
620+
state = mPlaybackState;
621+
}
619622
long duration = -1;
620623
if (mMetadata != null && mMetadata.containsKey(MediaMetadata.METADATA_KEY_DURATION)) {
621624
duration = mMetadata.getLong(MediaMetadata.METADATA_KEY_DURATION);
@@ -674,7 +677,8 @@ public void destroy() {
674677

675678
@Override
676679
public void sendEvent(String event, Bundle data) {
677-
mHandler.post(MessageHandler.MSG_SEND_EVENT, event, data);
680+
mHandler.post(MessageHandler.MSG_SEND_EVENT, event,
681+
data == null ? null : new Bundle(data));
678682
}
679683

680684
@Override
@@ -712,7 +716,11 @@ public void setLaunchPendingIntent(PendingIntent pi) {
712716

713717
@Override
714718
public void setMetadata(MediaMetadata metadata) {
715-
mMetadata = metadata;
719+
// Make a copy of the metadata as the underlying bundle may be
720+
// modified on this thread.
721+
synchronized (mLock) {
722+
mMetadata = metadata == null ? null : new MediaMetadata.Builder(metadata).build();
723+
}
716724
mHandler.post(MessageHandler.MSG_UPDATE_METADATA);
717725
}
718726

@@ -723,14 +731,18 @@ public void setPlaybackState(PlaybackState state) {
723731
if (MediaSession.isActiveState(oldState) && newState == PlaybackState.STATE_PAUSED) {
724732
mLastActiveTime = SystemClock.elapsedRealtime();
725733
}
726-
mPlaybackState = state;
734+
synchronized (mLock) {
735+
mPlaybackState = state;
736+
}
727737
mService.onSessionPlaystateChange(MediaSessionRecord.this, oldState, newState);
728738
mHandler.post(MessageHandler.MSG_UPDATE_PLAYBACK_STATE);
729739
}
730740

731741
@Override
732742
public void setQueue(ParceledListSlice queue) {
733-
mQueue = queue;
743+
synchronized (mLock) {
744+
mQueue = queue;
745+
}
734746
mHandler.post(MessageHandler.MSG_UPDATE_QUEUE);
735747
}
736748

@@ -742,7 +754,9 @@ public void setQueueTitle(CharSequence title) {
742754

743755
@Override
744756
public void setExtras(Bundle extras) {
745-
mExtras = extras;
757+
synchronized (mLock) {
758+
mExtras = extras == null ? null : new Bundle(extras);
759+
}
746760
mHandler.post(MessageHandler.MSG_UPDATE_EXTRAS);
747761
}
748762

@@ -1118,7 +1132,9 @@ public void sendCustomAction(String action, Bundle args)
11181132

11191133
@Override
11201134
public MediaMetadata getMetadata() {
1121-
return mMetadata;
1135+
synchronized (mLock) {
1136+
return mMetadata;
1137+
}
11221138
}
11231139

11241140
@Override
@@ -1128,7 +1144,9 @@ public PlaybackState getPlaybackState() {
11281144

11291145
@Override
11301146
public ParceledListSlice getQueue() {
1131-
return mQueue;
1147+
synchronized (mLock) {
1148+
return mQueue;
1149+
}
11321150
}
11331151

11341152
@Override
@@ -1138,7 +1156,9 @@ public CharSequence getQueueTitle() {
11381156

11391157
@Override
11401158
public Bundle getExtras() {
1141-
return mExtras;
1159+
synchronized (mLock) {
1160+
return mExtras;
1161+
}
11421162
}
11431163

11441164
@Override

0 commit comments

Comments
 (0)