From ba6f8c8bef19111cd6247df87392149b7963c36f Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 9 Nov 2016 12:25:44 -0700 Subject: 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. (cherry picked from commit 850c83e6da3b34a4eab804133420247fc9dbb8a1) Test: builds, boots, runs with minimal logs triggered Bug: 32715088 Merged-In: I70aa2666ae21dae8f09ded2063bed359c0b210c5 Change-Id: Ic2be10dc8478d613cea6e2c976b9987c250b86b7 --- .../java/com/android/internal/os/TransferPipe.java | 30 ++++++++++++++-------- location/java/android/location/IFusedProvider.aidl | 4 +-- .../android/server/InputMethodManagerService.java | 19 +++++++------- .../com/android/server/NetworkScoreService.java | 11 ++++---- .../server/location/LocationProviderProxy.java | 13 ++++------ .../server/pm/EphemeralResolverConnection.java | 13 +++++----- .../android/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 e76b395e9e2d..f9041507ffdd 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 8870d2a2a420..e86ad1ac5147 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 df1b6f51bc33..3ad264d38bb5 100644 --- a/services/core/java/com/android/server/InputMethodManagerService.java +++ b/services/core/java/com/android/server/InputMethodManagerService.java @@ -25,6 +25,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; @@ -4030,9 +4031,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."); @@ -4046,9 +4047,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); } } @@ -4056,9 +4057,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 4e969bef864f..4c9ea58e6395 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -46,8 +46,10 @@ import com.android.internal.R; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; 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; @@ -439,12 +441,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 5eb06ed20b42..b44087c07c16 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 68b465aaf816..07d98bc0077a 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; @@ -85,13 +88,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 07b26e83e934..6b919df290bb 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); } } } -- cgit v1.2.3-59-g8ed1b