fix race in vehicle hal mocking

- run mock start / stop in main thread of car service so that
  it does not have race with on-going init
- fix synchronization in CarTestManager so that callback
  and mock start / stop cannot deadlock

bug: 27245898
Change-Id: Iceb0b46227368ad5ead9f70f5a977155367f96b2

3 files changed, 145 insertions, 32 deletions

diff --git a/car-systemtest-lib/src/android/car/test/CarTestManager.java b/car-systemtest-lib/src/android/car/test/CarTestManager.java
index df448944b2..89f55ad516 100644
--- a/car-systemtest-lib/src/android/car/test/CarTestManager.java
+++ b/car-systemtest-lib/src/android/car/test/CarTestManager.java
@@ -66,7 +66,7 @@ public class CarTestManager implements CarManagerBase {
         // should not happen for embedded
     }
 
-    public synchronized void injectEvent(VehiclePropValue value) {
+    public void injectEvent(VehiclePropValue value) {
         try {
             mService.injectEvent(new VehiclePropValueParcelable(value));
         } catch (RemoteException e) {
@@ -80,24 +80,30 @@ public class CarTestManager implements CarManagerBase {
      * @param mock
      * @flags Combination of FLAG_MOCKING_*
      */
-    public synchronized void startMocking(VehicleNetworkHalMock mock, int flags) {
-        mHalMock = mock;
-        mHalMockImpl = new IVehicleNetworkHalMockImpl(this);
+    public void startMocking(VehicleNetworkHalMock mock, int flags) {
+        IVehicleNetworkHalMockImpl halMockImpl = new IVehicleNetworkHalMockImpl(this);
+        synchronized (this) {
+            mHalMock = mock;
+            mHalMockImpl = halMockImpl;
+        }
         try {
-            mService.startMocking(mHalMockImpl, flags);
+            mService.startMocking(halMockImpl, flags);
         } catch (RemoteException e) {
             handleRemoteException(e);
         }
     }
 
-    public synchronized void stopMocking() {
+    public void stopMocking() {
+        IVehicleNetworkHalMockImpl halMockImpl;
+        synchronized (this) {
+            halMockImpl = mHalMockImpl;
+            mHalMock = null;
+            mHalMockImpl = null;
+        }
         try {
-            mService.stopMocking(mHalMockImpl);
+            mService.stopMocking(halMockImpl);
         } catch (RemoteException e) {
             handleRemoteException(e);
-        } finally {
-            mHalMock = null;
-            mHalMockImpl = null;
         }
     }
 
diff --git a/service/src/com/android/car/CarServiceUtils.java b/service/src/com/android/car/CarServiceUtils.java
index 3070ff966b..fba04705ad 100644
--- a/service/src/com/android/car/CarServiceUtils.java
+++ b/service/src/com/android/car/CarServiceUtils.java
@@ -21,13 +21,18 @@ import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.os.Binder;
+import android.os.Handler;
+import android.os.Looper;
 import android.util.Log;
 
 /** Utility class */
-public class CarServiceUtils {
+public final class CarServiceUtils {
 
     private static final String PACKAGE_NOT_FOUND = "Package not found:";
 
+    /** do not construct. static only */
+    private CarServiceUtils() {};
+
     /**
      * Check if package name passed belongs to UID for the current binder call.
      * @param context
@@ -56,4 +61,81 @@ public class CarServiceUtils {
                     ", The package does not belong to caller's uid:" + uid);
         }
     }
+
+    /**
+     * Execute a runnable on the main thread
+     *
+     * @param action The code to run on the main thread.
+     */
+    public static void runOnMain(Runnable action) {
+        runOnLooper(Looper.getMainLooper(), action);
+    }
+
+    /**
+     * Execute a runnable in the given looper
+     * @param looper Looper to run the action.
+     * @param action The code to run.
+     */
+    public static void runOnLooper(Looper looper, Runnable action) {
+        new Handler(looper).post(action);
+    }
+
+    /**
+     * Execute a call on the application's main thread, blocking until it is
+     * complete.  Useful for doing things that are not thread-safe, such as
+     * looking at or modifying the view hierarchy.
+     *
+     * @param action The code to run on the main thread.
+     */
+    public static void runOnMainSync(Runnable action) {
+        runOnLooperSync(Looper.getMainLooper(), action);
+    }
+
+    /**
+     * Execute a call on the given Looper thread, blocking until it is
+     * complete.
+     *
+     * @param looper Looper to run the action.
+     * @param action The code to run on the main thread.
+     */
+    public static void runOnLooperSync(Looper looper, Runnable action) {
+        if (Looper.myLooper() == looper) {
+            // requested thread is the same as the current thread. call directly.
+            action.run();
+        } else {
+            Handler handler = new Handler(looper);
+            SyncRunnable sr = new SyncRunnable(action);
+            handler.post(sr);
+            sr.waitForComplete();
+        }
+    }
+
+    private static final class SyncRunnable implements Runnable {
+        private final Runnable mTarget;
+        private volatile boolean mComplete = false;
+
+        public SyncRunnable(Runnable target) {
+            mTarget = target;
+        }
+
+        @Override
+        public void run() {
+            mTarget.run();
+            synchronized (this) {
+                mComplete = true;
+                notifyAll();
+            }
+        }
+
+        public void waitForComplete() {
+            synchronized (this) {
+                while (!mComplete) {
+                    try {
+                        wait();
+                    } catch (InterruptedException e) {
+                    }
+                }
+            }
+        }
+    }
 }
diff --git a/service/src/com/android/car/CarTestService.java b/service/src/com/android/car/CarTestService.java
index 59a12f46c5..2752e3051d 100644
--- a/service/src/com/android/car/CarTestService.java
+++ b/service/src/com/android/car/CarTestService.java
@@ -39,6 +39,7 @@ public class CarTestService extends ICarTest.Stub implements CarServiceBase {
     private final ICarImpl mICarImpl;
     private boolean mInMocking = false;
     private int mMockingFlags = 0;
+    private Exception mException = null;
 
     public CarTestService(Context context, ICarImpl carImpl) {
         mContext = context;
@@ -71,34 +72,58 @@ public class CarTestService extends ICarTest.Stub implements CarServiceBase {
     }
 
     @Override
-    public void startMocking(IVehicleNetworkHalMock mock, int flags) {
+    public void startMocking(final IVehicleNetworkHalMock mock, final int flags) {
         ICarImpl.assertVehicleHalMockPermission(mContext);
-        try {
-            mVehicleNetwork.startMocking(mock);
-            VehicleHal.getInstance().startMocking();
-            mICarImpl.startMocking();
-            synchronized (this) {
-                mInMocking = true;
-                mMockingFlags = flags;
+        CarServiceUtils.runOnMainSync(new Runnable() {
+            @Override
+            public void run() {
+                try {
+                    mVehicleNetwork.startMocking(mock);
+                    VehicleHal.getInstance().startMocking();
+                    mICarImpl.startMocking();
+                    synchronized (this) {
+                        mInMocking = true;
+                        mMockingFlags = flags;
+                        mException = null;
+                    }
+                } catch (Exception e) {
+                    Log.w(CarLog.TAG_TEST, "startMocking failed", e);
+                    synchronized (this) {
+                        mException = e;
+                    }
+                }
+                Log.i(CarLog.TAG_TEST, "start vehicle HAL mocking, flags:0x" +
+                        Integer.toHexString(flags));
+            }
+        });
+        synchronized (this) {
+            if (mException != null) {
+                throw new IllegalStateException(mException);
             }
-        } catch (Exception e) {
-            Log.w(CarLog.TAG_TEST, "startMocking failed", e);
-            throw e;
         }
-        Log.i(CarLog.TAG_TEST, "start vehicle HAL mocking, flags:0x" + Integer.toHexString(flags));
     }
 
     @Override
-    public void stopMocking(IVehicleNetworkHalMock mock) {
+    public void stopMocking(final IVehicleNetworkHalMock mock) {
         ICarImpl.assertVehicleHalMockPermission(mContext);
-        mVehicleNetwork.stopMocking(mock);
-        VehicleHal.getInstance().stopMocking();
-        mICarImpl.stopMocking();
-        synchronized (this) {
-            mInMocking = false;
-            mMockingFlags = 0;
-        }
-        Log.i(CarLog.TAG_TEST, "stop vehicle HAL mocking");
+        CarServiceUtils.runOnMainSync(new Runnable() {
+            @Override
+            public void run() {
+                try {
+                    mVehicleNetwork.stopMocking(mock);
+                    VehicleHal.getInstance().stopMocking();
+                    mICarImpl.stopMocking();
+                    synchronized (this) {
+                        mInMocking = false;
+                        mMockingFlags = 0;
+                        mException = null;
+                    }
+                    Log.i(CarLog.TAG_TEST, "stop vehicle HAL mocking");
+                } catch (Exception e) {
+                    Log.w(CarLog.TAG_TEST, "stopMocking failed", e);
+                }
+            }
+        });
     }
 
     public synchronized boolean isInMocking() {