From a3aec61861b8d1f8e95454b7411f4181614f07b3 Mon Sep 17 00:00:00 2001
From: Gyumin Sim <gyumin@google.com>
Date: Tue, 1 Jun 2021 13:23:51 +0900
Subject: [PATCH] Ensure MediaSession2Record is initialized before used

NPE happens at mController.close() if session is destroyed while the
MediaController2 instance is being created in the constructor of
MediaSession2Record. It ensures the MediaSession2Record instance is
initialized before the instance is used by Controller2Callback.

Bug: 189119798
Fix: 189119798
Test: atest CtsMediaTestCases:android.media.cts.MediaSessionTest
Change-Id: I23ca2f3dbc657ac38f4154a0fb428a4c25e6b5ad
---
 .../server/media/MediaSession2Record.java     | 32 +++++++++++++------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/services/core/java/com/android/server/media/MediaSession2Record.java b/services/core/java/com/android/server/media/MediaSession2Record.java
index 162c388dadd1e..607218e20ea8e 100644
--- a/services/core/java/com/android/server/media/MediaSession2Record.java
+++ b/services/core/java/com/android/server/media/MediaSession2Record.java
@@ -56,13 +56,17 @@ public class MediaSession2Record implements MediaSessionRecordImpl {
 
     public MediaSession2Record(Session2Token sessionToken, MediaSessionService service,
             Looper handlerLooper, int policies) {
-        mSessionToken = sessionToken;
-        mService = service;
-        mHandlerExecutor = new HandlerExecutor(new Handler(handlerLooper));
-        mController = new MediaController2.Builder(service.getContext(), sessionToken)
-                .setControllerCallback(mHandlerExecutor, new Controller2Callback())
-                .build();
-        mPolicies = policies;
+        // The lock is required to prevent `Controller2Callback` from using partially initialized
+        // `MediaSession2Record.this`.
+        synchronized (mLock) {
+            mSessionToken = sessionToken;
+            mService = service;
+            mHandlerExecutor = new HandlerExecutor(new Handler(handlerLooper));
+            mController = new MediaController2.Builder(service.getContext(), sessionToken)
+                    .setControllerCallback(mHandlerExecutor, new Controller2Callback())
+                    .build();
+            mPolicies = policies;
+        }
     }
 
     @Override
@@ -176,10 +180,12 @@ public class MediaSession2Record implements MediaSessionRecordImpl {
             if (DEBUG) {
                 Log.d(TAG, "connected to " + mSessionToken + ", allowed=" + allowedCommands);
             }
+            MediaSessionService service;
             synchronized (mLock) {
                 mIsConnected = true;
+                service = mService;
             }
-            mService.onSessionActiveStateChanged(MediaSession2Record.this);
+            service.onSessionActiveStateChanged(MediaSession2Record.this);
         }
 
         @Override
@@ -187,10 +193,12 @@ public class MediaSession2Record implements MediaSessionRecordImpl {
             if (DEBUG) {
                 Log.d(TAG, "disconnected from " + mSessionToken);
             }
+            MediaSessionService service;
             synchronized (mLock) {
                 mIsConnected = false;
+                service = mService;
             }
-            mService.onSessionDied(MediaSession2Record.this);
+            service.onSessionDied(MediaSession2Record.this);
         }
 
         @Override
@@ -199,7 +207,11 @@ public class MediaSession2Record implements MediaSessionRecordImpl {
                 Log.d(TAG, "playback active changed, " + mSessionToken + ", active="
                         + playbackActive);
             }
-            mService.onSessionPlaybackStateChanged(MediaSession2Record.this, playbackActive);
+            MediaSessionService service;
+            synchronized (mLock) {
+                service = mService;
+            }
+            service.onSessionPlaybackStateChanged(MediaSession2Record.this, playbackActive);
         }
     }
 }