diff options
| -rw-r--r-- | core/java/android/os/BugreportManager.java | 23 | ||||
| -rw-r--r-- | services/core/java/com/android/server/os/BugreportManagerServiceImpl.java | 78 |
2 files changed, 90 insertions, 11 deletions
diff --git a/core/java/android/os/BugreportManager.java b/core/java/android/os/BugreportManager.java index 89b6577c79ba..bc979b4b9b31 100644 --- a/core/java/android/os/BugreportManager.java +++ b/core/java/android/os/BugreportManager.java @@ -27,6 +27,8 @@ import android.content.Context; import com.android.internal.util.Preconditions; +import libcore.io.IoUtils; + import java.io.FileDescriptor; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -123,6 +125,8 @@ public class BugreportManager { * <p>The bugreport artifacts will be copied over to the given file descriptors only if the * user consents to sharing with the calling app. * + * <p>{@link BugreportManager} takes ownership of {@code bugreportFd} and {@code screenshotFd}. + * * @param bugreportFd file to write the bugreport. This should be opened in write-only, * append mode. * @param screenshotFd file to write the screenshot, if necessary. This should be opened @@ -136,12 +140,13 @@ public class BugreportManager { @NonNull BugreportParams params, @NonNull @CallbackExecutor Executor executor, @NonNull BugreportCallback callback) { - Preconditions.checkNotNull(bugreportFd); - Preconditions.checkNotNull(params); - Preconditions.checkNotNull(executor); - Preconditions.checkNotNull(callback); - DumpstateListener dsListener = new DumpstateListener(executor, callback); try { + Preconditions.checkNotNull(bugreportFd); + Preconditions.checkNotNull(params); + Preconditions.checkNotNull(executor); + Preconditions.checkNotNull(callback); + + DumpstateListener dsListener = new DumpstateListener(executor, callback); // Note: mBinder can get callingUid from the binder transaction. mBinder.startBugreport(-1 /* callingUid */, mContext.getOpPackageName(), @@ -151,6 +156,12 @@ public class BugreportManager { params.getMode(), dsListener); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); + } finally { + // We can close the file descriptors here because binder would have duped them. + IoUtils.closeQuietly(bugreportFd); + if (screenshotFd != null) { + IoUtils.closeQuietly(screenshotFd); + } } } @@ -170,7 +181,7 @@ public class BugreportManager { private final Executor mExecutor; private final BugreportCallback mCallback; - DumpstateListener(Executor executor, @Nullable BugreportCallback callback) { + DumpstateListener(Executor executor, BugreportCallback callback) { mExecutor = executor; mCallback = callback; } diff --git a/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java b/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java index f4454ae2a180..653b65c22d7b 100644 --- a/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java +++ b/services/core/java/com/android/server/os/BugreportManagerServiceImpl.java @@ -38,10 +38,6 @@ import com.android.internal.util.Preconditions; import java.io.FileDescriptor; -// TODO(b/111441001): -// Intercept onFinished() & implement death recipient here and shutdown -// bugreportd service. - /** * Implementation of the service that provides a privileged API to capture and consume bugreports. * @@ -157,9 +153,12 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { reportError(listener, IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR); return; } + + // Wrap the listener so we can intercept binder events directly. + IDumpstateListener myListener = new DumpstateListener(listener, ds); try { ds.startBugreport(callingUid, callingPackage, - bugreportFd, screenshotFd, bugreportMode, listener); + bugreportFd, screenshotFd, bugreportMode, myListener); } catch (RemoteException e) { reportError(listener, IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR); } @@ -226,4 +225,73 @@ class BugreportManagerServiceImpl extends IDumpstate.Stub { Slog.w(TAG, message); throw new IllegalArgumentException(message); } + + + private final class DumpstateListener extends IDumpstateListener.Stub + implements DeathRecipient { + private final IDumpstateListener mListener; + private final IDumpstate mDs; + private boolean mDone = false; + + DumpstateListener(IDumpstateListener listener, IDumpstate ds) { + mListener = listener; + mDs = ds; + try { + mDs.asBinder().linkToDeath(this, 0); + } catch (RemoteException e) { + Slog.e(TAG, "Unable to register Death Recipient for IDumpstate", e); + } + } + + @Override + public void onProgress(int progress) throws RemoteException { + mListener.onProgress(progress); + } + + @Override + public void onError(int errorCode) throws RemoteException { + synchronized (mLock) { + mDone = true; + } + mListener.onError(errorCode); + } + + @Override + public void onFinished() throws RemoteException { + synchronized (mLock) { + mDone = true; + } + mListener.onFinished(); + } + + @Override + public void binderDied() { + synchronized (mLock) { + if (!mDone) { + // If we have not gotten a "done" callback this must be a crash. + Slog.e(TAG, "IDumpstate likely crashed. Notifying listener"); + try { + mListener.onError(IDumpstateListener.BUGREPORT_ERROR_RUNTIME_ERROR); + } catch (RemoteException ignored) { + // If listener is not around, there isn't anything to do here. + } + } + } + mDs.asBinder().unlinkToDeath(this, 0); + } + + // Old methods; unused in the API flow. + @Override + public void onProgressUpdated(int progress) throws RemoteException { + } + + @Override + public void onMaxProgressUpdated(int maxProgress) throws RemoteException { + } + + @Override + public void onSectionComplete(String title, int status, int size, int durationMs) + throws RemoteException { + } + } } |