Binding on-demand #11: Dump TransportClients

And adjust logs.

Change-Id: I04fcfb77bac14db36cc30288878bbb65332deb81
Ref: http://go/br-binding-on-demand
Bug: 17140907
Test: adb shell dumpsys backup

diff --git a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
index 5188910..05b01d3 100644 (file)
--- a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
+++ b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
@@ -3514,6 +3514,9 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
                     } else if ("agents".startsWith(arg)) {
                         dumpAgents(pw);
                         return;
+                    } else if ("transportclients".equals(arg.toLowerCase())) {
+                        mTransportManager.dump(pw);
+                        return;
                     }
                 }
             }
@@ -3576,6 +3579,8 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
                 }
             }
 
+            mTransportManager.dump(pw);
+
             pw.println("Pending init: " + mPendingInits.size());
             for (String s : mPendingInits) {
                 pw.println("    " + s);

diff --git a/services/backup/java/com/android/server/backup/TransportManager.java b/services/backup/java/com/android/server/backup/TransportManager.java
index 5b901ee..e17bdc3 100644 (file)
--- a/services/backup/java/com/android/server/backup/TransportManager.java
+++ b/services/backup/java/com/android/server/backup/TransportManager.java
@@ -42,6 +42,7 @@ import com.android.server.backup.transport.TransportConnectionListener;
 import com.android.server.backup.transport.TransportNotAvailableException;
 import com.android.server.backup.transport.TransportNotRegisteredException;
 
+import java.io.PrintWriter;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -632,6 +633,10 @@ public class TransportManager {
                 !Thread.holdsLock(mTransportLock), "Can't call transport with transport lock held");
     }
 
+    public void dump(PrintWriter pw) {
+        mTransportClientManager.dump(pw);
+    }
+
     private static Predicate<ComponentName> fromPackageFilter(String packageName) {
         return transportComponent -> packageName.equals(transportComponent.getPackageName());
     }

diff --git a/services/backup/java/com/android/server/backup/transport/TransportClient.java b/services/backup/java/com/android/server/backup/transport/TransportClient.java
index 399f338..7b2e3df 100644 (file)
--- a/services/backup/java/com/android/server/backup/transport/TransportClient.java
+++ b/services/backup/java/com/android/server/backup/transport/TransportClient.java
@@ -16,6 +16,8 @@
 
 package com.android.server.backup.transport;
 
+import static com.android.server.backup.transport.TransportUtils.formatMessage;
+
 import android.annotation.IntDef;
 import android.annotation.Nullable;
 import android.annotation.WorkerThread;
@@ -28,6 +30,7 @@ import android.os.Handler;
 import android.os.IBinder;
 import android.os.Looper;
 import android.os.UserHandle;
+import android.text.format.DateFormat;
 import android.util.ArrayMap;
 import android.util.EventLog;
 import android.util.Log;
@@ -41,6 +44,9 @@ import com.android.server.backup.TransportManager;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
@@ -65,6 +71,7 @@ import java.util.concurrent.ExecutionException;
  */
 public class TransportClient {
     private static final String TAG = "TransportClient";
+    private static final int LOG_BUFFER_SIZE = 5;
 
     private final Context mContext;
     private final Intent mBindIntent;
@@ -73,6 +80,10 @@ public class TransportClient {
     private final Handler mListenerHandler;
     private final String mPrefixForLog;
     private final Object mStateLock = new Object();
+    private final Object mLogBufferLock = new Object();
+
+    @GuardedBy("mLogBufferLock")
+    private final List<String> mLogBuffer = new LinkedList<>();
 
     @GuardedBy("mStateLock")
     private final Map<TransportConnectionListener, String> mListeners = new ArrayMap<>();
@@ -112,7 +123,7 @@ public class TransportClient {
 
         // For logging
         String classNameForLog = mTransportComponent.getShortClassName().replaceFirst(".*\\.", "");
-        mPrefixForLog = classNameForLog + "#" + mIdentifier + ": ";
+        mPrefixForLog = classNameForLog + "#" + mIdentifier + ":";
     }
 
     public ComponentName getTransportComponent() {
@@ -229,7 +240,7 @@ public class TransportClient {
 
             switch (mState) {
                 case State.UNUSABLE:
-                    log(Log.DEBUG, caller, "Async connect: UNUSABLE client");
+                    log(Log.WARN, caller, "Async connect: UNUSABLE client");
                     notifyListener(listener, null, caller);
                     break;
                 case State.IDLE:
@@ -324,14 +335,14 @@ public class TransportClient {
 
         IBackupTransport transport = mTransport;
         if (transport != null) {
-            log(Log.DEBUG, caller, "Sync connect: reusing transport");
+            log(Log.INFO, caller, "Sync connect: reusing transport");
             return transport;
         }
 
         // If it's already UNUSABLE we return straight away, no need to go to main-thread
         synchronized (mStateLock) {
             if (mState == State.UNUSABLE) {
-                log(Log.DEBUG, caller, "Sync connect: UNUSABLE client");
+                log(Log.WARN, caller, "Sync connect: UNUSABLE client");
                 return null;
             }
         }
@@ -403,13 +414,16 @@ public class TransportClient {
     }
 
     private void notifyListener(
-            TransportConnectionListener listener, IBackupTransport transport, String caller) {
-        log(Log.VERBOSE, caller, "Notifying listener of transport = " + transport);
+            TransportConnectionListener listener,
+            @Nullable IBackupTransport transport,
+            String caller) {
+        String transportString = (transport != null) ? "IBackupTransport" : "null";
+        log(Log.INFO, "Notifying [" + caller + "] transport = " + transportString);
         mListenerHandler.post(() -> listener.onTransportConnectionResult(transport, this));
     }
 
     @GuardedBy("mStateLock")
-    private void notifyListenersAndClearLocked(IBackupTransport transport) {
+    private void notifyListenersAndClearLocked(@Nullable IBackupTransport transport) {
         for (Map.Entry<TransportConnectionListener, String> entry : mListeners.entrySet()) {
             TransportConnectionListener listener = entry.getKey();
             String caller = entry.getValue();
@@ -509,13 +523,30 @@ public class TransportClient {
     }
 
     private void log(int priority, String message) {
-        TransportUtils.log(priority, TAG, message);
+        TransportUtils.log(priority, TAG, formatMessage(mPrefixForLog, null, message));
+        saveLogEntry(formatMessage(null, null, message));
     }
 
-    private void log(int priority, String caller, String msg) {
-        TransportUtils.log(priority, TAG, mPrefixForLog, caller, msg);
-        // TODO(brufino): Log in internal list for dump
-        // CharSequence time = DateFormat.format("yyyy-MM-dd HH:mm:ss", System.currentTimeMillis());
+    private void log(int priority, String caller, String message) {
+        TransportUtils.log(priority, TAG, formatMessage(mPrefixForLog, caller, message));
+        saveLogEntry(formatMessage(null, caller, message));
+    }
+
+    private void saveLogEntry(String message) {
+        CharSequence time = DateFormat.format("yyyy-MM-dd HH:mm:ss", System.currentTimeMillis());
+        message = time + " " + message;
+        synchronized (mLogBufferLock) {
+            if (mLogBuffer.size() == LOG_BUFFER_SIZE) {
+                mLogBuffer.remove(mLogBuffer.size() - 1);
+            }
+            mLogBuffer.add(0, message);
+        }
+    }
+
+    List<String> getLogBuffer() {
+        synchronized (mLogBufferLock) {
+            return Collections.unmodifiableList(mLogBuffer);
+        }
     }
 
     @IntDef({Transition.DOWN, Transition.NO_TRANSITION, Transition.UP})

diff --git a/services/backup/java/com/android/server/backup/transport/TransportClientManager.java b/services/backup/java/com/android/server/backup/transport/TransportClientManager.java
index 1cbe747..1132bce 100644 (file)
--- a/services/backup/java/com/android/server/backup/transport/TransportClientManager.java
+++ b/services/backup/java/com/android/server/backup/transport/TransportClientManager.java
@@ -17,19 +17,20 @@
 package com.android.server.backup.transport;
 
 import static com.android.server.backup.TransportManager.SERVICE_ACTION_TRANSPORT_HOST;
+import static com.android.server.backup.transport.TransportUtils.formatMessage;
 
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.util.Log;
-
 import com.android.server.backup.TransportManager;
+import java.io.PrintWriter;
+import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * Manages the creation and disposal of {@link TransportClient}s. The only class that should use
  * this is {@link TransportManager}, all the other usages should go to {@link TransportManager}.
- *
- * <p>TODO(brufino): Implement pool of TransportClients
  */
 public class TransportClientManager {
     private static final String TAG = "TransportClientManager";
@@ -37,6 +38,7 @@ public class TransportClientManager {
     private final Context mContext;
     private final Object mTransportClientsLock = new Object();
     private int mTransportClientsCreated = 0;
+    private Map<TransportClient, String> mTransportClientsCallerMap = new WeakHashMap<>();
 
     public TransportClientManager(Context context) {
         mContext = context;
@@ -62,8 +64,10 @@ public class TransportClientManager {
                             bindIntent,
                             transportComponent,
                             Integer.toString(mTransportClientsCreated));
+            mTransportClientsCallerMap.put(transportClient, caller);
             mTransportClientsCreated++;
-            TransportUtils.log(Log.DEBUG, TAG, caller, "Retrieving " + transportClient);
+            TransportUtils.log(
+                    Log.DEBUG, TAG, formatMessage(null, caller, "Retrieving " + transportClient));
             return transportClient;
         }
     }
@@ -77,7 +81,25 @@ public class TransportClientManager {
      *     details.
      */
     public void disposeOfTransportClient(TransportClient transportClient, String caller) {
-        TransportUtils.log(Log.DEBUG, TAG, caller, "Disposing of " + transportClient);
         transportClient.unbind(caller);
+        synchronized (mTransportClientsLock) {
+            TransportUtils.log(
+                    Log.DEBUG, TAG, formatMessage(null, caller, "Disposing of " + transportClient));
+            mTransportClientsCallerMap.remove(transportClient);
+        }
+    }
+
+    public void dump(PrintWriter pw) {
+        pw.println("Transport clients created: " + mTransportClientsCreated);
+        synchronized (mTransportClientsLock) {
+            pw.println("Current transport clients: " + mTransportClientsCallerMap.size());
+            for (TransportClient transportClient : mTransportClientsCallerMap.keySet()) {
+                String caller = mTransportClientsCallerMap.get(transportClient);
+                pw.println("    " + transportClient + " [" + caller + "]");
+                for (String logEntry : transportClient.getLogBuffer()) {
+                    pw.println("        " + logEntry);
+                }
+            }
+        }
     }
 }

diff --git a/services/backup/java/com/android/server/backup/transport/TransportUtils.java b/services/backup/java/com/android/server/backup/transport/TransportUtils.java
index 92bba9b..56b2d44 100644 (file)
--- a/services/backup/java/com/android/server/backup/transport/TransportUtils.java
+++ b/services/backup/java/com/android/server/backup/transport/TransportUtils.java
@@ -41,21 +41,20 @@ public class TransportUtils {
     }
 
     static void log(int priority, String tag, String message) {
-        log(priority, tag, null, message);
-    }
-
-    static void log(int priority, String tag, @Nullable String caller, String message) {
-        log(priority, tag, "", caller, message);
+        if (Log.isLoggable(tag, priority)) {
+            Slog.println(priority, tag, message);
+        }
     }
 
-    static void log(
-            int priority, String tag, String prefix, @Nullable String caller, String message) {
-        if (Log.isLoggable(tag, priority)) {
-            if (caller != null) {
-                prefix += "[" + caller + "] ";
-            }
-            Slog.println(priority, tag, prefix + message);
+    static String formatMessage(@Nullable String prefix, @Nullable String caller, String message) {
+        StringBuilder string = new StringBuilder();
+        if (prefix != null) {
+            string.append(prefix).append(" ");
+        }
+        if (caller != null) {
+            string.append("[").append(caller).append("] ");
         }
+        return string.append(message).toString();
     }
 
     private TransportUtils() {}