From a25ac6d49b49a9208015ef805d305eb5cbbb8798 Mon Sep 17 00:00:00 2001
From: Soonil Nagarkar <sooniln@google.com>
Date: Mon, 10 Jan 2022 12:14:34 -0800
Subject: Ensure location provider exceptions are visible

Exceptions thrown on one-way binder threads are dropped and ignored,
which is very harmful to writing stable code. Rethrow any exceptions
from these binder calls on the main thread to crash the process.

Test: presubmits
Change-Id: Ie5986061c6d664f59b83679c90dd07adad1a7ec2
---
 .../location/provider/LocationProviderBase.java    | 40 +++++++++++++++++-----
 1 file changed, 31 insertions(+), 9 deletions(-)

(limited to 'location/java')

diff --git a/location/java/android/location/provider/LocationProviderBase.java b/location/java/android/location/provider/LocationProviderBase.java
index 88a24794411c..529edddf4cf0 100644
--- a/location/java/android/location/provider/LocationProviderBase.java
+++ b/location/java/android/location/provider/LocationProviderBase.java
@@ -26,7 +26,9 @@ import android.content.Context;
 import android.content.Intent;
 import android.location.Location;
 import android.os.Bundle;
+import android.os.Handler;
 import android.os.IBinder;
+import android.os.Looper;
 import android.os.RemoteException;
 import android.util.Log;
 
@@ -308,9 +310,7 @@ public abstract class LocationProviderBase {
             synchronized (mBinder) {
                 try {
                     manager.onInitialize(mAllowed, mProperties, mAttributionTag);
-                } catch (RemoteException e) {
-                    throw e.rethrowFromSystemServer();
-                } catch (RuntimeException e) {
+                } catch (RemoteException | RuntimeException e) {
                     Log.w(mTag, e);
                 }
 
@@ -320,12 +320,28 @@ public abstract class LocationProviderBase {
 
         @Override
         public void setRequest(ProviderRequest request) {
-            onSetRequest(request);
+            try {
+                onSetRequest(request);
+            } catch (RuntimeException e) {
+                // exceptions on one-way binder threads are dropped - move to a different thread
+                Log.w(mTag, e);
+                new Handler(Looper.getMainLooper()).post(() -> {
+                    throw new AssertionError(e);
+                });
+            }
         }
 
         @Override
         public void flush() {
-            onFlush(this::onFlushComplete);
+            try {
+                onFlush(this::onFlushComplete);
+            } catch (RuntimeException e) {
+                // exceptions on one-way binder threads are dropped - move to a different thread
+                Log.w(mTag, e);
+                new Handler(Looper.getMainLooper()).post(() -> {
+                    throw new AssertionError(e);
+                });
+            }
         }
 
         private void onFlushComplete() {
@@ -333,9 +349,7 @@ public abstract class LocationProviderBase {
             if (manager != null) {
                 try {
                     manager.onFlushComplete();
-                } catch (RemoteException e) {
-                    throw e.rethrowFromSystemServer();
-                } catch (RuntimeException e) {
+                } catch (RemoteException | RuntimeException e) {
                     Log.w(mTag, e);
                 }
             }
@@ -343,7 +357,15 @@ public abstract class LocationProviderBase {
 
         @Override
         public void sendExtraCommand(String command, Bundle extras) {
-            onSendExtraCommand(command, extras);
+            try {
+                onSendExtraCommand(command, extras);
+            } catch (RuntimeException e) {
+                // exceptions on one-way binder threads are dropped - move to a different thread
+                Log.w(mTag, e);
+                new Handler(Looper.getMainLooper()).post(() -> {
+                    throw new AssertionError(e);
+                });
+            }
         }
     }
 }
-- 
cgit v1.2.3-59-g8ed1b