Fix GuardedBy error prone warnings
Note: In CarDevicePolicyService.java, the GuardedBy was referring to inexistent sLock instead of mLock.
Removed lambda functions and simplified logic as per the guideline at go/aaos-platform-guideline.
Bug: 258809108
Test: atest CtsCarTestCases CarSecurityPermissionTest
Test: atest CarServiceUnitTest CarServiceTest
Change-Id: If3aaece38ba95bec816d9d1aa4ecf38b19e60557
diff --git a/car-lib/src/android/car/Car.java b/car-lib/src/android/car/Car.java
index 6085ac9..ef90abf 100644
--- a/car-lib/src/android/car/Car.java
+++ b/car-lib/src/android/car/Car.java
@@ -2247,7 +2247,8 @@
@GuardedBy("mLock")
private void handleCarDisconnectLocked() {
if (mConnectionState == STATE_DISCONNECTED) {
- // can happen when client calls disconnect with onServiceDisconnected already called.
+ // can happen when client calls disconnect with onServiceDisconnected already
+ // called.
return;
}
mEventHandler.removeCallbacks(mConnectionRetryRunnable);
@@ -2756,7 +2757,9 @@
// Experimental or non-existing
String className = null;
try {
- className = mService.getCarManagerClassForFeature(serviceName);
+ synchronized (mLock) {
+ className = mService.getCarManagerClassForFeature(serviceName);
+ }
} catch (RemoteException e) {
handleRemoteExceptionFromCarService(e);
return null;
@@ -2813,9 +2816,10 @@
}
}
+ @GuardedBy("mLock")
private void tearDownCarManagersLocked() {
// All disconnected handling should be only doing its internal cleanup.
- for (CarManagerBase manager: mServiceMap.values()) {
+ for (CarManagerBase manager : mServiceMap.values()) {
manager.onCarDisconnected();
}
mServiceMap.clear();
diff --git a/car-lib/src/android/car/drivingstate/CarDrivingStateManager.java b/car-lib/src/android/car/drivingstate/CarDrivingStateManager.java
index d37627e..087925f 100644
--- a/car-lib/src/android/car/drivingstate/CarDrivingStateManager.java
+++ b/car-lib/src/android/car/drivingstate/CarDrivingStateManager.java
@@ -261,7 +261,7 @@
return;
}
CarDrivingStateEventListener listener;
- synchronized (this) {
+ synchronized (mLock) {
listener = mDrvStateEventListener;
}
if (listener != null) {
diff --git a/car-lib/src/android/car/evs/CarEvsManager.java b/car-lib/src/android/car/evs/CarEvsManager.java
index ae8c495..7738619 100644
--- a/car-lib/src/android/car/evs/CarEvsManager.java
+++ b/car-lib/src/android/car/evs/CarEvsManager.java
@@ -417,11 +417,10 @@
Objects.requireNonNull(listener);
Objects.requireNonNull(executor);
- if (mStatusListener != null) {
- throw new IllegalStateException("A status listener is already registered.");
- }
-
synchronized (mStatusLock) {
+ if (mStatusListener != null) {
+ throw new IllegalStateException("A status listener is already registered.");
+ }
mStatusListener = listener;
mStatusListenerExecutor = executor;
}
diff --git a/car-lib/src/android/car/util/concurrent/AndroidFuture.java b/car-lib/src/android/car/util/concurrent/AndroidFuture.java
index a1ee23f..9adc97e 100644
--- a/car-lib/src/android/car/util/concurrent/AndroidFuture.java
+++ b/car-lib/src/android/car/util/concurrent/AndroidFuture.java
@@ -233,10 +233,12 @@
private void callListenerAsync(BiConsumer<? super T, ? super Throwable> listener,
@Nullable T res, @Nullable Throwable err) {
- if (mListenerExecutor == DIRECT_EXECUTOR) {
- callListener(listener, res, err);
- } else {
- mListenerExecutor.execute(() -> callListener(listener, res, err));
+ synchronized (mLock) {
+ if (mListenerExecutor == DIRECT_EXECUTOR) {
+ callListener(listener, res, err);
+ } else {
+ mListenerExecutor.execute(() -> callListener(listener, res, err));
+ }
}
}
diff --git a/car-lib/src/android/car/vms/VmsSubscriptionHelper.java b/car-lib/src/android/car/vms/VmsSubscriptionHelper.java
index 0af5c35..04f4c29 100644
--- a/car-lib/src/android/car/vms/VmsSubscriptionHelper.java
+++ b/car-lib/src/android/car/vms/VmsSubscriptionHelper.java
@@ -25,12 +25,9 @@
import com.android.internal.annotations.GuardedBy;
import java.util.Collections;
-import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Consumer;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
/**
* Internal utility for computing subscription updates.
@@ -43,10 +40,10 @@
private final Object mLock = new Object();
@GuardedBy("mLock")
- private final Set<VmsLayer> mLayerSubscriptions = new ArraySet<>();
+ private final ArraySet<VmsLayer> mLayerSubscriptions = new ArraySet<>();
@GuardedBy("mLock")
- private final Map<VmsLayer, SparseBooleanArray> mPublisherSubscriptions = new ArrayMap<>();
+ private final ArrayMap<VmsLayer, SparseBooleanArray> mPublisherSubscriptions = new ArrayMap<>();
@GuardedBy("mLock")
private boolean mPendingUpdate;
@@ -129,14 +126,22 @@
*/
@NonNull
@AddedInOrBefore(majorVersion = 33)
+ @GuardedBy("mLock")
public Set<VmsAssociatedLayer> getSubscriptions() {
- return Stream.concat(
- mLayerSubscriptions.stream().map(
- layer -> new VmsAssociatedLayer(layer, Collections.emptySet())),
- mPublisherSubscriptions.entrySet().stream()
- .filter(entry -> !mLayerSubscriptions.contains(entry.getKey()))
- .map(VmsSubscriptionHelper::toAssociatedLayer))
- .collect(Collectors.toSet());
+ Set<VmsAssociatedLayer> vmsAssociatedLayerSet = new ArraySet<>();
+ for (int i = 0; i < mLayerSubscriptions.size(); i++) {
+ VmsLayer layer = mLayerSubscriptions.valueAt(i);
+ vmsAssociatedLayerSet.add(new VmsAssociatedLayer(layer, Collections.emptySet()));
+ }
+
+ for (int i = 0; i < mPublisherSubscriptions.size(); i++) {
+ VmsLayer layer = mPublisherSubscriptions.keyAt(i);
+ if (!mLayerSubscriptions.contains(layer)) {
+ vmsAssociatedLayerSet.add(
+ toAssociatedLayer(layer, mPublisherSubscriptions.valueAt(i)));
+ }
+ }
+ return vmsAssociatedLayerSet;
}
private void publishSubscriptionUpdate() {
@@ -148,13 +153,12 @@
}
}
- private static VmsAssociatedLayer toAssociatedLayer(
- Map.Entry<VmsLayer, SparseBooleanArray> entry) {
- SparseBooleanArray providerIdArray = entry.getValue();
+ private static VmsAssociatedLayer toAssociatedLayer(VmsLayer layer,
+ SparseBooleanArray providerIdArray) {
Set<Integer> providerIds = new ArraySet<>(providerIdArray.size());
for (int i = 0; i < providerIdArray.size(); i++) {
providerIds.add(providerIdArray.keyAt(i));
}
- return new VmsAssociatedLayer(entry.getKey(), providerIds);
+ return new VmsAssociatedLayer(layer, providerIds);
}
}
diff --git a/car-lib/src/android/car/watchdog/CarWatchdogManager.java b/car-lib/src/android/car/watchdog/CarWatchdogManager.java
index 1977dd8..d80aee1 100644
--- a/car-lib/src/android/car/watchdog/CarWatchdogManager.java
+++ b/car-lib/src/android/car/watchdog/CarWatchdogManager.java
@@ -827,6 +827,7 @@
}
}
+ @GuardedBy("mLock")
private boolean checkConditionLocked() {
if (mRemainingConditions < 0) {
Log.wtf(TAG, "Remaining condition is less than zero: should not happen");
@@ -1049,12 +1050,14 @@
}
public int resourceOveruseFlag() {
- int flag = 0;
- for (int i = 0; i < mNumListenersByResource.size(); ++i) {
- flag |= mNumListenersByResource.valueAt(i) > 0 ? mNumListenersByResource.keyAt(i)
- : 0;
+ synchronized (mLock) {
+ int flag = 0;
+ for (int i = 0; i < mNumListenersByResource.size(); ++i) {
+ flag |= mNumListenersByResource.valueAt(i) > 0 ? mNumListenersByResource.keyAt(
+ i) : 0;
+ }
+ return flag;
}
- return flag;
}
}
diff --git a/service/src/com/android/car/ICarImpl.java b/service/src/com/android/car/ICarImpl.java
index 08330b5..a57fa83 100644
--- a/service/src/com/android/car/ICarImpl.java
+++ b/service/src/com/android/car/ICarImpl.java
@@ -987,8 +987,10 @@
dumpService(service, writer);
}
}
- if (mCarTestService != null) {
- dumpService(mCarTestService, writer);
+ synchronized (mLock) {
+ if (mCarTestService != null) {
+ dumpService(mCarTestService, writer);
+ }
}
}
diff --git a/service/src/com/android/car/admin/CarDevicePolicyService.java b/service/src/com/android/car/admin/CarDevicePolicyService.java
index 3dd3fc9..94ffe39 100644
--- a/service/src/com/android/car/admin/CarDevicePolicyService.java
+++ b/service/src/com/android/car/admin/CarDevicePolicyService.java
@@ -95,7 +95,7 @@
})
public @interface NewUserDisclaimerStatus {}
- @GuardedBy("sLock")
+ @GuardedBy("mLock")
private final SparseIntArray mUserDisclaimerStatusPerUser = new SparseIntArray();
private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {
diff --git a/service/src/com/android/car/evs/CarEvsService.java b/service/src/com/android/car/evs/CarEvsService.java
index efccd73..b9d3452 100644
--- a/service/src/com/android/car/evs/CarEvsService.java
+++ b/service/src/com/android/car/evs/CarEvsService.java
@@ -992,7 +992,9 @@
public void stopActivity() {
CarServiceUtils.assertPermission(mContext, Car.PERMISSION_REQUEST_CAR_EVS_ACTIVITY);
- mStateEngine.execute(REQUEST_PRIORITY_NORMAL, SERVICE_STATE_INACTIVE, mStreamCallback);
+ synchronized (mLock) {
+ mStateEngine.execute(REQUEST_PRIORITY_NORMAL, SERVICE_STATE_INACTIVE, mStreamCallback);
+ }
}
/**
diff --git a/service/src/com/android/car/power/PowerComponentHandler.java b/service/src/com/android/car/power/PowerComponentHandler.java
index 1bc9151..c5e4917 100644
--- a/service/src/com/android/car/power/PowerComponentHandler.java
+++ b/service/src/com/android/car/power/PowerComponentHandler.java
@@ -363,12 +363,14 @@
try (BufferedWriter writer = new BufferedWriter(
new OutputStreamWriter(fos, StandardCharsets.UTF_8))) {
- for (int i = 0; i < mComponentsOffByPolicy.size(); i++) {
- if (!mComponentsOffByPolicy.valueAt(i)) {
- continue;
+ synchronized (mLock) {
+ for (int i = 0; i < mComponentsOffByPolicy.size(); i++) {
+ if (!mComponentsOffByPolicy.valueAt(i)) {
+ continue;
+ }
+ writer.write(powerComponentToString(mComponentsOffByPolicy.keyAt(i)));
+ writer.newLine();
}
- writer.write(powerComponentToString(mComponentsOffByPolicy.keyAt(i)));
- writer.newLine();
}
writer.flush();
mOffComponentsByUserFile.finishWrite(fos);
diff --git a/service/src/com/android/car/user/CarUserService.java b/service/src/com/android/car/user/CarUserService.java
index c11741a..92d89f9 100644
--- a/service/src/com/android/car/user/CarUserService.java
+++ b/service/src/com/android/car/user/CarUserService.java
@@ -2781,11 +2781,11 @@
}
private void notifyLegacyUserSwitch(@UserIdInt int fromUserId, @UserIdInt int toUserId) {
- if (DBG) {
- Slogf.d(TAG, "notifyLegacyUserSwitch(%d, %d): mUserIdForUserSwitchInProcess=%d",
- fromUserId, toUserId, mUserIdForUserSwitchInProcess);
- }
synchronized (mLockUser) {
+ if (DBG) {
+ Slogf.d(TAG, "notifyLegacyUserSwitch(%d, %d): mUserIdForUserSwitchInProcess=%d",
+ fromUserId, toUserId, mUserIdForUserSwitchInProcess);
+ }
if (mUserIdForUserSwitchInProcess != USER_NULL) {
if (mUserIdForUserSwitchInProcess == toUserId) {
if (DBG) {
diff --git a/service/src/com/android/car/vms/VmsClientInfo.java b/service/src/com/android/car/vms/VmsClientInfo.java
index 492ccc8..2481f6a 100644
--- a/service/src/com/android/car/vms/VmsClientInfo.java
+++ b/service/src/com/android/car/vms/VmsClientInfo.java
@@ -125,14 +125,13 @@
}
}
+ @GuardedBy("mLock")
Collection<VmsLayersOffering> getAllOfferings() {
List<VmsLayersOffering> result = new ArrayList<>(mOfferings.size());
- synchronized (mLock) {
- for (int i = 0; i < mOfferings.size(); i++) {
- int providerId = mOfferings.keyAt(i);
- Set<VmsLayerDependency> providerOfferings = mOfferings.valueAt(i);
- result.add(new VmsLayersOffering(new ArraySet<>(providerOfferings), providerId));
- }
+ for (int i = 0; i < mOfferings.size(); i++) {
+ int providerId = mOfferings.keyAt(i);
+ Set<VmsLayerDependency> providerOfferings = mOfferings.valueAt(i);
+ result.add(new VmsLayersOffering(new ArraySet<>(providerOfferings), providerId));
}
return result;
}
