Speculative fix for concurrency SessionManager issue

Change the constraints for the session cleanup runnable to not
synchronize on the global instance. Instead, synchronize on the
Handler object to remove pressure on the hot path.

Flag: com.android.internal.telephony.flags.ims_resolver_user_aware
Bug: 375088362
Test: atest TelecomUnitTests
Change-Id: I65b82c1c8b74367d5611f6c3582a27073a2af6f2

2 files changed, 39 insertions, 14 deletions

diff --git a/telecomm/java/android/telecom/Log.java b/telecomm/java/android/telecom/Log.java
index a34094ce6452..98949d0c45cf 100644
--- a/telecomm/java/android/telecom/Log.java
+++ b/telecomm/java/android/telecom/Log.java
@@ -68,7 +68,7 @@ public class Log {
     // Used to synchronize singleton logging lazy initialization
     private static final Object sSingletonSync = new Object();
     private static EventManager sEventManager;
-    private static SessionManager sSessionManager;
+    private static volatile SessionManager sSessionManager;
     private static Object sLock = null;
 
     /**
@@ -379,6 +379,23 @@ public class Log {
         return sSessionManager;
     }
 
+    @VisibleForTesting
+    public static SessionManager setSessionManager(Context context,
+            java.lang.Runnable cleanSessionRunnable) {
+        // Checking for null again outside of synchronization because we only need to synchronize
+        // during the lazy loading of the session logger. We don't need to synchronize elsewhere.
+        if (sSessionManager == null) {
+            synchronized (sSingletonSync) {
+                if (sSessionManager == null) {
+                    sSessionManager = new SessionManager(cleanSessionRunnable);
+                    sSessionManager.setContext(context);
+                    return sSessionManager;
+                }
+            }
+        }
+        return sSessionManager;
+    }
+
     public static void setTag(String tag) {
         TAG = tag;
         DEBUG = isLoggable(android.util.Log.DEBUG);
diff --git a/telecomm/java/android/telecom/Logging/SessionManager.java b/telecomm/java/android/telecom/Logging/SessionManager.java
index 00e344c67cc5..ac1e69e92ec0 100644
--- a/telecomm/java/android/telecom/Logging/SessionManager.java
+++ b/telecomm/java/android/telecom/Logging/SessionManager.java
@@ -62,9 +62,7 @@ public class SessionManager {
 
     @VisibleForTesting
     public final ConcurrentHashMap<Integer, Session> mSessionMapper = new ConcurrentHashMap<>(64);
-    @VisibleForTesting
-    public java.lang.Runnable mCleanStaleSessions = () ->
-            cleanupStaleSessions(getSessionCleanupTimeoutMs());
+    private final java.lang.Runnable mCleanStaleSessions;
     private final Handler mSessionCleanupHandler = new Handler(Looper.getMainLooper());
 
     // Overridden in LogTest to skip query to ContentProvider
@@ -110,29 +108,39 @@ public class SessionManager {
     }
 
     public SessionManager() {
+        mCleanStaleSessions = () -> cleanupStaleSessions(getSessionCleanupTimeoutMs());
+    }
+
+    @VisibleForTesting
+    public SessionManager(java.lang.Runnable cleanStaleSessionsRunnable) {
+        mCleanStaleSessions = cleanStaleSessionsRunnable;
     }
 
     private long getSessionCleanupTimeoutMs() {
         return mSessionCleanupTimeoutMs.get();
     }
 
-    private synchronized void resetStaleSessionTimer() {
+    private void resetStaleSessionTimer() {
         if (!Flags.endSessionImprovements()) {
-            mSessionCleanupHandler.removeCallbacksAndMessages(null);
-            // Will be null in Log Testing
-            if (mCleanStaleSessions != null) {
-                mSessionCleanupHandler.postDelayed(mCleanStaleSessions,
-                        getSessionCleanupTimeoutMs());
-            }
-        } else {
-            if (mCleanStaleSessions != null
-                    && !mSessionCleanupHandler.hasCallbacks(mCleanStaleSessions)) {
+            resetStaleSessionTimerOld();
+            return;
+        }
+        // Will be null in Log Testing
+        if (mCleanStaleSessions == null) return;
+        synchronized (mSessionCleanupHandler) {
+            if (!mSessionCleanupHandler.hasCallbacks(mCleanStaleSessions)) {
                 mSessionCleanupHandler.postDelayed(mCleanStaleSessions,
                         getSessionCleanupTimeoutMs());
             }
         }
     }
 
+    private synchronized void resetStaleSessionTimerOld() {
+        if (mCleanStaleSessions == null) return;
+        mSessionCleanupHandler.removeCallbacksAndMessages(null);
+        mSessionCleanupHandler.postDelayed(mCleanStaleSessions, getSessionCleanupTimeoutMs());
+    }
+
     /**
      * Determines whether or not to start a new session or continue an existing session based on
      * the {@link Session.Info} info passed into startSession. If info is null, a new Session is