From 850c83e6da3b34a4eab804133420247fc9dbb8a1 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 9 Nov 2016 12:25:44 -0700 Subject: [PATCH] Move dump() to dumpAsync(), more oneway calls. When calling out to dump services hosted by external apps, use dumpAsync() to avoid hanging if the remote process is wedged. Test: builds, boots, runs with minimal logs triggered Bug: 32715088 Change-Id: I70aa2666ae21dae8f09ded2063bed359c0b210c5 --- .../com/android/internal/os/TransferPipe.java | 30 ++++++++++++------- .../java/android/location/IFusedProvider.aidl | 4 +-- .../server/InputMethodManagerService.java | 19 ++++++------ .../android/server/NetworkScoreService.java | 11 ++++--- .../location/LocationProviderProxy.java | 13 ++++---- .../pm/EphemeralResolverConnection.java | 13 ++++---- .../server/print/RemotePrintSpooler.java | 13 ++++---- 7 files changed, 56 insertions(+), 47 deletions(-) diff --git a/core/java/com/android/internal/os/TransferPipe.java b/core/java/com/android/internal/os/TransferPipe.java index e76b395e9e2d4..f9041507ffddc 100644 --- a/core/java/com/android/internal/os/TransferPipe.java +++ b/core/java/com/android/internal/os/TransferPipe.java @@ -16,6 +16,7 @@ package com.android.internal.os; +import java.io.Closeable; import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -32,13 +33,13 @@ import android.util.Slog; /** * Helper for transferring data through a pipe from a client app. */ -public final class TransferPipe implements Runnable { +public final class TransferPipe implements Runnable, Closeable { static final String TAG = "TransferPipe"; static final boolean DEBUG = false; static final long DEFAULT_TIMEOUT = 5000; // 5 seconds - final Thread mThread;; + final Thread mThread; final ParcelFileDescriptor[] mFds; FileDescriptor mOutFd; @@ -54,8 +55,13 @@ public final class TransferPipe implements Runnable { } public TransferPipe() throws IOException { + this(null); + } + + public TransferPipe(String bufferPrefix) throws IOException { mThread = new Thread(this, "TransferPipe"); mFds = ParcelFileDescriptor.createPipe(); + mBufferPrefix = bufferPrefix; } ParcelFileDescriptor getReadFd() { @@ -70,6 +76,11 @@ public final class TransferPipe implements Runnable { mBufferPrefix = prefix; } + public static void dumpAsync(IBinder binder, FileDescriptor out, String[] args) + throws IOException, RemoteException { + goDump(binder, out, args); + } + static void go(Caller caller, IInterface iface, FileDescriptor out, String prefix, String[] args) throws IOException, RemoteException { go(caller, iface, out, prefix, args, DEFAULT_TIMEOUT); @@ -86,12 +97,9 @@ public final class TransferPipe implements Runnable { return; } - TransferPipe tp = new TransferPipe(); - try { + try (TransferPipe tp = new TransferPipe()) { caller.go(iface, tp.getWriteFd().getFileDescriptor(), prefix, args); tp.go(out, timeout); - } finally { - tp.kill(); } } @@ -111,12 +119,9 @@ public final class TransferPipe implements Runnable { return; } - TransferPipe tp = new TransferPipe(); - try { + try (TransferPipe tp = new TransferPipe()) { binder.dumpAsync(tp.getWriteFd().getFileDescriptor(), args); tp.go(out, timeout); - } finally { - tp.kill(); } } @@ -173,6 +178,11 @@ public final class TransferPipe implements Runnable { } } + @Override + public void close() { + kill(); + } + public void kill() { synchronized (this) { closeFd(0); diff --git a/location/java/android/location/IFusedProvider.aidl b/location/java/android/location/IFusedProvider.aidl index 8870d2a2a4206..e86ad1ac5147a 100644 --- a/location/java/android/location/IFusedProvider.aidl +++ b/location/java/android/location/IFusedProvider.aidl @@ -22,11 +22,11 @@ import android.hardware.location.IFusedLocationHardware; * Interface definition for Location providers that require FLP services. * @hide */ -interface IFusedProvider { +oneway interface IFusedProvider { /** * Provides access to a FusedLocationHardware instance needed for the provider to work. * * @param instance The FusedLocationHardware available for the provider to use. */ void onFusedLocationHardwareChange(in IFusedLocationHardware instance); -} \ No newline at end of file +} diff --git a/services/core/java/com/android/server/InputMethodManagerService.java b/services/core/java/com/android/server/InputMethodManagerService.java index 2698f9546357d..2bf5866e22f54 100644 --- a/services/core/java/com/android/server/InputMethodManagerService.java +++ b/services/core/java/com/android/server/InputMethodManagerService.java @@ -29,6 +29,7 @@ import com.android.internal.inputmethod.InputMethodUtils; import com.android.internal.inputmethod.InputMethodUtils.InputMethodSettings; import com.android.internal.os.HandlerCaller; import com.android.internal.os.SomeArgs; +import com.android.internal.os.TransferPipe; import com.android.internal.util.FastXmlSerializer; import com.android.internal.view.IInputContext; import com.android.internal.view.IInputMethod; @@ -4041,9 +4042,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub if (client != null) { pw.flush(); try { - client.client.asBinder().dump(fd, args); - } catch (RemoteException e) { - p.println("Input method client dead: " + e); + TransferPipe.dumpAsync(client.client.asBinder(), fd, args); + } catch (IOException | RemoteException e) { + p.println("Failed to dump input method client: " + e); } } else { p.println("No input method client."); @@ -4057,9 +4058,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub p.println(" "); pw.flush(); try { - focusedWindowClient.client.asBinder().dump(fd, args); - } catch (RemoteException e) { - p.println("Input method client in focused window dead: " + e); + TransferPipe.dumpAsync(focusedWindowClient.client.asBinder(), fd, args); + } catch (IOException | RemoteException e) { + p.println("Failed to dump input method client in focused window: " + e); } } @@ -4067,9 +4068,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub if (method != null) { pw.flush(); try { - method.asBinder().dump(fd, args); - } catch (RemoteException e) { - p.println("Input method service dead: " + e); + TransferPipe.dumpAsync(method.asBinder(), fd, args); + } catch (IOException | RemoteException e) { + p.println("Failed to dump input method service: " + e); } } else { p.println("No input method service."); diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index 83d6344c95b0d..f5cda0ab4d632 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -42,8 +42,10 @@ import android.util.Log; import com.android.internal.R; import com.android.internal.annotations.GuardedBy; import com.android.internal.content.PackageMonitor; +import com.android.internal.os.TransferPipe; import java.io.FileDescriptor; +import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; import java.util.HashMap; @@ -418,12 +420,9 @@ public class NetworkScoreService extends INetworkScoreService.Stub { for (INetworkScoreCache scoreCache : getScoreCaches()) { try { - scoreCache.asBinder().dump(fd, args); - } catch (RemoteException e) { - writer.println("Unable to dump score cache"); - if (Log.isLoggable(TAG, Log.VERBOSE)) { - Log.v(TAG, "Unable to dump score cache", e); - } + TransferPipe.dumpAsync(scoreCache.asBinder(), fd, args); + } catch (IOException | RemoteException e) { + writer.println("Failed to dump score cache: " + e); } } if (mServiceConnection != null) { diff --git a/services/core/java/com/android/server/location/LocationProviderProxy.java b/services/core/java/com/android/server/location/LocationProviderProxy.java index 5eb06ed20b42c..b44087c07c169 100644 --- a/services/core/java/com/android/server/location/LocationProviderProxy.java +++ b/services/core/java/com/android/server/location/LocationProviderProxy.java @@ -17,6 +17,7 @@ package com.android.server.location; import java.io.FileDescriptor; +import java.io.IOException; import java.io.PrintWriter; import android.content.Context; @@ -30,6 +31,7 @@ import android.util.Log; import com.android.internal.location.ProviderProperties; import com.android.internal.location.ILocationProvider; import com.android.internal.location.ProviderRequest; +import com.android.internal.os.TransferPipe; import com.android.server.LocationManagerService; import com.android.server.ServiceWatcher; @@ -230,14 +232,9 @@ public class LocationProviderProxy implements LocationProviderInterface { pw.flush(); try { - service.asBinder().dump(fd, args); - } catch (RemoteException e) { - pw.println("service down (RemoteException)"); - Log.w(TAG, e); - } catch (Exception e) { - pw.println("service down (Exception)"); - // never let remote service crash system server - Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e); + TransferPipe.dumpAsync(service.asBinder(), fd, args); + } catch (IOException | RemoteException e) { + pw.println("Failed to dump location provider: " + e); } } diff --git a/services/core/java/com/android/server/pm/EphemeralResolverConnection.java b/services/core/java/com/android/server/pm/EphemeralResolverConnection.java index a6a377419a6a3..27e2a6e03ae5c 100644 --- a/services/core/java/com/android/server/pm/EphemeralResolverConnection.java +++ b/services/core/java/com/android/server/pm/EphemeralResolverConnection.java @@ -32,7 +32,10 @@ import android.os.SystemClock; import android.os.UserHandle; import android.util.TimedRemoteCaller; +import com.android.internal.os.TransferPipe; + import java.io.FileDescriptor; +import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; @@ -86,13 +89,11 @@ final class EphemeralResolverConnection { .append((mRemoteInstance != null) ? "true" : "false").println(); pw.flush(); - try { - getRemoteInstanceLazy().asBinder().dump(fd, new String[] { prefix }); - } catch (TimeoutException te) { - /* ignore */ - } catch (RemoteException re) { - /* ignore */ + TransferPipe.dumpAsync(getRemoteInstanceLazy().asBinder(), fd, + new String[] { prefix }); + } catch (IOException | TimeoutException | RemoteException e) { + pw.println("Failed to dump remote instance: " + e); } } } diff --git a/services/print/java/com/android/server/print/RemotePrintSpooler.java b/services/print/java/com/android/server/print/RemotePrintSpooler.java index 07b26e83e934d..6b919df290bb7 100644 --- a/services/print/java/com/android/server/print/RemotePrintSpooler.java +++ b/services/print/java/com/android/server/print/RemotePrintSpooler.java @@ -43,9 +43,12 @@ import android.printservice.PrintService; import android.util.Slog; import android.util.TimedRemoteCaller; +import com.android.internal.os.TransferPipe; + import libcore.io.IoUtils; import java.io.FileDescriptor; +import java.io.IOException; import java.io.PrintWriter; import java.lang.ref.WeakReference; import java.util.List; @@ -572,13 +575,11 @@ final class RemotePrintSpooler { .append((mRemoteInstance != null) ? "true" : "false").println(); pw.flush(); - try { - getRemoteInstanceLazy().asBinder().dump(fd, new String[]{prefix}); - } catch (TimeoutException te) { - /* ignore */ - } catch (RemoteException re) { - /* ignore */ + TransferPipe.dumpAsync(getRemoteInstanceLazy().asBinder(), fd, + new String[] { prefix }); + } catch (IOException | TimeoutException | RemoteException e) { + pw.println("Failed to dump remote instance: " + e); } } }