Made AccessPoint's scan result list thread safe
If wifi verbose logging is enabled, the main thread may iterate through
the scan result list while the worker thread updates it, resulting in
an ArrayIndexOutOfBounds exception. This change adds a lock to every
iteration through mScanResults and mExtraScanResults.
Bug: 134098126
Test: atest WifiTrackerTest && atest AccessPointTest
Change-Id: If42a9ad8031ff66b1732a79772ba441185f7a18f
diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java
index e28c612..fdba64c 100644
--- a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java
+++ b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java
@@ -55,6 +55,7 @@
import android.util.Log;
import android.util.Pair;
+import androidx.annotation.GuardedBy;
import androidx.annotation.NonNull;
import com.android.internal.annotations.VisibleForTesting;
@@ -108,6 +109,14 @@
/** The key which identifies this AccessPoint grouping. */
private String mKey;
+ /**
+ * Synchronization lock for managing concurrency between main and worker threads.
+ *
+ * <p>This lock should be held for all modifications to {@link #mScanResults} and
+ * {@link #mExtraScanResults}.
+ */
+ private final Object mLock = new Object();
+
@IntDef({Speed.NONE, Speed.SLOW, Speed.MODERATE, Speed.FAST, Speed.VERY_FAST})
@Retention(RetentionPolicy.SOURCE)
public @interface Speed {
@@ -134,12 +143,14 @@
}
/** The underlying set of scan results comprising this AccessPoint. */
+ @GuardedBy("mLock")
private final ArraySet<ScanResult> mScanResults = new ArraySet<>();
/**
* Extra set of unused scan results corresponding to this AccessPoint for verbose logging
* purposes, such as a set of Passpoint roaming scan results when home scans are available.
*/
+ @GuardedBy("mLock")
private final ArraySet<ScanResult> mExtraScanResults = new ArraySet<>();
/**
@@ -489,8 +500,10 @@
if (isVerboseLoggingEnabled()) {
builder.append(",rssi=").append(mRssi);
- builder.append(",scan cache size=").append(mScanResults.size()
- + mExtraScanResults.size());
+ synchronized (mLock) {
+ builder.append(",scan cache size=").append(mScanResults.size()
+ + mExtraScanResults.size());
+ }
}
return builder.append(')').toString();
@@ -532,18 +545,20 @@
*/
private boolean updateScores(WifiNetworkScoreCache scoreCache, long maxScoreCacheAgeMillis) {
long nowMillis = SystemClock.elapsedRealtime();
- for (ScanResult result : mScanResults) {
- ScoredNetwork score = scoreCache.getScoredNetwork(result);
- if (score == null) {
- continue;
- }
- TimestampedScoredNetwork timedScore = mScoredNetworkCache.get(result.BSSID);
- if (timedScore == null) {
- mScoredNetworkCache.put(
- result.BSSID, new TimestampedScoredNetwork(score, nowMillis));
- } else {
- // Update data since the has been seen in the score cache
- timedScore.update(score, nowMillis);
+ synchronized (mLock) {
+ for (ScanResult result : mScanResults) {
+ ScoredNetwork score = scoreCache.getScoredNetwork(result);
+ if (score == null) {
+ continue;
+ }
+ TimestampedScoredNetwork timedScore = mScoredNetworkCache.get(result.BSSID);
+ if (timedScore == null) {
+ mScoredNetworkCache.put(
+ result.BSSID, new TimestampedScoredNetwork(score, nowMillis));
+ } else {
+ // Update data since the has been seen in the score cache
+ timedScore.update(score, nowMillis);
+ }
}
}
@@ -619,12 +634,14 @@
mIsScoredNetworkMetered |= score.meteredHint;
}
} else {
- for (ScanResult result : mScanResults) {
- ScoredNetwork score = scoreCache.getScoredNetwork(result);
- if (score == null) {
- continue;
+ synchronized (mLock) {
+ for (ScanResult result : mScanResults) {
+ ScoredNetwork score = scoreCache.getScoredNetwork(result);
+ if (score == null) {
+ continue;
+ }
+ mIsScoredNetworkMetered |= score.meteredHint;
}
- mIsScoredNetworkMetered |= score.meteredHint;
}
}
return oldMetering == mIsScoredNetworkMetered;
@@ -741,8 +758,10 @@
*/
public Set<ScanResult> getScanResults() {
Set<ScanResult> allScans = new ArraySet<>();
- allScans.addAll(mScanResults);
- allScans.addAll(mExtraScanResults);
+ synchronized (mLock) {
+ allScans.addAll(mScanResults);
+ allScans.addAll(mExtraScanResults);
+ }
return allScans;
}
@@ -766,10 +785,12 @@
ScanResult bestResult = null;
int bestRssi = UNREACHABLE_RSSI;
- for (ScanResult result : mScanResults) {
- if (result.level > bestRssi) {
- bestRssi = result.level;
- bestResult = result;
+ synchronized (mLock) {
+ for (ScanResult result : mScanResults) {
+ if (result.level > bestRssi) {
+ bestRssi = result.level;
+ bestResult = result;
+ }
}
}
@@ -1211,9 +1232,11 @@
savedState.putInt(KEY_EAPTYPE, mEapType);
if (mConfig != null) savedState.putParcelable(KEY_CONFIG, mConfig);
savedState.putParcelable(KEY_WIFIINFO, mInfo);
- savedState.putParcelableArray(KEY_SCANRESULTS,
- mScanResults.toArray(new Parcelable[mScanResults.size()
- + mExtraScanResults.size()]));
+ synchronized (mLock) {
+ savedState.putParcelableArray(KEY_SCANRESULTS,
+ mScanResults.toArray(new Parcelable[mScanResults.size()
+ + mExtraScanResults.size()]));
+ }
savedState.putParcelableArrayList(KEY_SCOREDNETWORKCACHE,
new ArrayList<>(mScoredNetworkCache.values()));
if (mNetworkInfo != null) {
@@ -1292,8 +1315,10 @@
}
int oldLevel = getLevel();
- mScanResults.clear();
- mScanResults.addAll(scanResults);
+ synchronized (mLock) {
+ mScanResults.clear();
+ mScanResults.addAll(scanResults);
+ }
updateBestRssiInfo();
int newLevel = getLevel();
@@ -1324,16 +1349,18 @@
void setScanResultsPasspoint(
@Nullable Collection<ScanResult> homeScans,
@Nullable Collection<ScanResult> roamingScans) {
- mExtraScanResults.clear();
- if (!CollectionUtils.isEmpty(homeScans)) {
- if (!CollectionUtils.isEmpty(roamingScans)) {
- mExtraScanResults.addAll(roamingScans);
+ synchronized (mLock) {
+ mExtraScanResults.clear();
+ if (!CollectionUtils.isEmpty(homeScans)) {
+ mIsRoaming = false;
+ if (!CollectionUtils.isEmpty(roamingScans)) {
+ mExtraScanResults.addAll(roamingScans);
+ }
+ setScanResults(homeScans);
+ } else if (!CollectionUtils.isEmpty(roamingScans)) {
+ mIsRoaming = true;
+ setScanResults(roamingScans);
}
- mIsRoaming = false;
- setScanResults(homeScans);
- } else if (!CollectionUtils.isEmpty(roamingScans)) {
- mIsRoaming = true;
- setScanResults(roamingScans);
}
}