Catch and log RuntimeExceptions thrown by VehicleHal and optionally propagate.
Changes:
- PropertyTimeout/RuntimeException on get() operation (metrics collection)
is caught and logged
- PropertyTimeout/RuntimeException on set() operation (sending VMS messages)
is caught, logged, and propagated on eng and userdebug builds as an
IllegalStateException
- SESSION_START handshakes are sent synchronously to HAL in response to
client handshake
- Updated VmsHalServiceTest to enable rethrow by default
- Tests added to validate error suppression, but no tests
added to validate rethrow as this would require major
refactoring due to VHAL operations being processed on a separate
thread.
Bug: 142879041
Bug: 137878135
Test: atest AndroidCarApiTest CarServiceTest CarServiceUnitTest
Test: Manual testing on Hawk of crash propagation w/ modified HAL
Change-Id: I6d615fa2764712444a1127bd52f21d3fa95b56e8
diff --git a/service/src/com/android/car/hal/VmsHalService.java b/service/src/com/android/car/hal/VmsHalService.java
index bba5d5f..99263d7 100644
--- a/service/src/com/android/car/hal/VmsHalService.java
+++ b/service/src/com/android/car/hal/VmsHalService.java
@@ -17,8 +17,6 @@
import static com.android.car.CarServiceUtils.toByteArray;
-import static java.lang.Integer.toHexString;
-
import android.car.VehicleAreaType;
import android.car.vms.IVmsPublisherClient;
import android.car.vms.IVmsPublisherService;
@@ -43,6 +41,7 @@
import android.hardware.automotive.vehicle.V2_0.VmsOfferingMessageIntegerValuesIndex;
import android.hardware.automotive.vehicle.V2_0.VmsPublisherInformationIntegerValuesIndex;
import android.hardware.automotive.vehicle.V2_0.VmsStartSessionMessageIntegerValuesIndex;
+import android.os.Build;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.IBinder;
@@ -54,7 +53,6 @@
import androidx.annotation.VisibleForTesting;
-import com.android.car.CarLog;
import com.android.car.vms.VmsClientManager;
import java.io.FileDescriptor;
@@ -87,6 +85,7 @@
private final int mCoreId;
private final MessageQueue mMessageQueue;
private final int mClientMetricsProperty;
+ private final boolean mPropagatePropertyException;
private volatile boolean mIsSupported = false;
private VmsClientManager mClientManager;
@@ -186,11 +185,7 @@
int messageType = msg.what;
VehiclePropValue vehicleProp = (VehiclePropValue) msg.obj;
if (DBG) Log.d(TAG, "Sending " + VmsMessageType.toString(messageType) + " message");
- try {
- setPropertyValue(vehicleProp);
- } catch (RemoteException e) {
- Log.e(TAG, "While sending " + VmsMessageType.toString(messageType));
- }
+ setPropertyValue(vehicleProp);
return true;
}
}
@@ -199,15 +194,17 @@
* Constructor used by {@link VehicleHal}
*/
VmsHalService(Context context, VehicleHal vehicleHal) {
- this(context, vehicleHal, SystemClock::uptimeMillis);
+ this(context, vehicleHal, SystemClock::uptimeMillis, (Build.IS_ENG || Build.IS_USERDEBUG));
}
@VisibleForTesting
- VmsHalService(Context context, VehicleHal vehicleHal, Supplier<Long> getCoreId) {
+ VmsHalService(Context context, VehicleHal vehicleHal, Supplier<Long> getCoreId,
+ boolean propagatePropertyException) {
mVehicleHal = vehicleHal;
mCoreId = (int) (getCoreId.get() % Integer.MAX_VALUE);
mMessageQueue = new MessageQueue();
mClientMetricsProperty = getClientMetricsProperty(context);
+ mPropagatePropertyException = propagatePropertyException;
}
private static int getClientMetricsProperty(Context context) {
@@ -326,8 +323,9 @@
VehiclePropValue vehicleProp = null;
try {
vehicleProp = mVehicleHal.get(mClientMetricsProperty);
- } catch (PropertyTimeoutException e) {
- Log.e(TAG, "Timeout while reading metrics from client");
+ } catch (PropertyTimeoutException | RuntimeException e) {
+ // Failures to retrieve metrics should be non-fatal
+ Log.e(TAG, "While reading metrics from client", e);
}
if (vehicleProp == null) {
if (DBG) Log.d(TAG, "Metrics unavailable");
@@ -395,7 +393,7 @@
Log.e(TAG, "Unexpected message type: " + messageType);
}
} catch (IndexOutOfBoundsException | RemoteException e) {
- Log.e(TAG, "While handling: " + messageType, e);
+ Log.e(TAG, "While handling " + VmsMessageType.toString(messageType), e);
}
}
}
@@ -425,9 +423,8 @@
mSubscriptionStateSequence = -1;
mAvailableLayersSequence = -1;
- // Enqueue an acknowledgement message
- mMessageQueue.enqueue(VmsMessageType.START_SESSION,
- createStartSessionMessage(mCoreId, clientId));
+ // Send acknowledgement message
+ setPropertyValue(createStartSessionMessage(mCoreId, clientId));
}
// Notify client manager of connection
@@ -678,7 +675,7 @@
mPublisherService.getSubscriptions()));
}
- private void setPropertyValue(VehiclePropValue vehicleProp) throws RemoteException {
+ private void setPropertyValue(VehiclePropValue vehicleProp) {
int messageType = vehicleProp.value.int32Values.get(
VmsBaseMessageIntegerValuesIndex.MESSAGE_TYPE);
@@ -690,11 +687,11 @@
try {
mVehicleHal.set(vehicleProp);
- } catch (PropertyTimeoutException e) {
- Log.e(CarLog.TAG_PROPERTY,
- "set, property not ready 0x" + toHexString(HAL_PROPERTY_ID));
- throw new RemoteException(
- "Timeout while sending " + VmsMessageType.toString(messageType));
+ } catch (PropertyTimeoutException | RuntimeException e) {
+ Log.e(TAG, "While sending " + VmsMessageType.toString(messageType), e.getCause());
+ if (mPropagatePropertyException) {
+ throw new IllegalStateException(e);
+ }
}
}
diff --git a/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java b/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
index 7571867..093ab9b 100644
--- a/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
@@ -17,7 +17,9 @@
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
@@ -44,8 +46,6 @@
import android.os.Binder;
import android.os.IBinder;
-import androidx.test.filters.RequiresDevice;
-
import com.android.car.R;
import com.android.car.test.utils.TemporaryFile;
import com.android.car.vms.VmsClientManager;
@@ -103,8 +103,13 @@
@Before
public void setUp() throws Exception {
+ initHalService(true);
+ }
+
+ private void initHalService(boolean propagatePropertyException) throws Exception {
when(mContext.getResources()).thenReturn(mResources);
- mHalService = new VmsHalService(mContext, mVehicleHal, () -> (long) CORE_ID);
+ mHalService = new VmsHalService(mContext, mVehicleHal, () -> (long) CORE_ID,
+ propagatePropertyException);
mHalService.setClientManager(mClientManager);
mHalService.setVmsSubscriberService(mSubscriberService);
@@ -158,6 +163,8 @@
0, // Sequence number
0)); // # of associated layers
+
+ waitForHandlerCompletion();
initOrder.verifyNoMoreInteractions();
reset(mClientManager, mSubscriberService, mVehicleHal);
}
@@ -165,7 +172,7 @@
@Test
public void testCoreId_IntegerOverflow() throws Exception {
mHalService = new VmsHalService(mContext, mVehicleHal,
- () -> (long) Integer.MAX_VALUE + CORE_ID);
+ () -> (long) Integer.MAX_VALUE + CORE_ID, true);
VehiclePropConfig propConfig = new VehiclePropConfig();
propConfig.prop = VehicleProperty.VEHICLE_MAP_SERVICE;
@@ -587,7 +594,6 @@
* </ul>
*/
@Test
- @RequiresDevice
public void testHandleStartSessionEvent() throws Exception {
when(mSubscriberService.getAvailableLayers()).thenReturn(
new VmsAvailableLayers(Collections.emptySet(), 5));
@@ -605,13 +611,18 @@
);
sendHalMessage(request);
+
InOrder inOrder = Mockito.inOrder(mClientManager, mVehicleHal);
inOrder.verify(mClientManager).onHalDisconnected();
inOrder.verify(mVehicleHal).set(response);
+ inOrder.verify(mClientManager).onHalConnected(mPublisherClient, mSubscriberClient);
+
+ waitForHandlerCompletion();
inOrder.verify(mVehicleHal).set(createHalMessage(
VmsMessageType.AVAILABILITY_CHANGE, // Message type
5, // Sequence number
0)); // # of associated layers
+
}
/**
@@ -962,6 +973,33 @@
}
@Test
+ public void testPropertySetExceptionNotPropagated_CoreStartSession() throws Exception {
+ doThrow(new RuntimeException()).when(mVehicleHal).set(any());
+ initHalService(false);
+
+ mHalService.init();
+ waitForHandlerCompletion();
+ }
+
+ @Test
+ public void testPropertySetExceptionNotPropagated_ClientStartSession() throws Exception {
+ initHalService(false);
+
+ when(mSubscriberService.getAvailableLayers()).thenReturn(
+ new VmsAvailableLayers(Collections.emptySet(), 0));
+ doThrow(new RuntimeException()).when(mVehicleHal).set(any());
+
+ VehiclePropValue request = createHalMessage(
+ VmsMessageType.START_SESSION, // Message type
+ -1, // Core ID (unknown)
+ CLIENT_ID // Client ID
+ );
+
+ sendHalMessage(request);
+ waitForHandlerCompletion();
+ }
+
+ @Test
public void testDumpMetrics_DefaultConfig() {
mHalService.dumpMetrics(new FileDescriptor());
verifyZeroInteractions(mVehicleHal);