From c12d33dbda4e4b142c8a034a963719d24c74425a Mon Sep 17 00:00:00 2001 From: Quang Luong Date: Thu, 26 Mar 2020 21:59:17 -0700 Subject: [WifiTrackerLib] Make WifiEntry scan result lists thread safe The scan result lists of each WifiEntry is modified by the worker thread, but clients on the main thread may need to iterate on the lists to generate the verbose log. This may result in concurrent modification exceptions, so we need to put locks on these lists. Bug: 150978717 Test: build Change-Id: Ice9f653b0496b20d87470ef0af1eda22a2d2fe9c --- .../com/android/wifitrackerlib/OsuWifiEntry.java | 12 ++++++-- .../android/wifitrackerlib/PasspointWifiEntry.java | 21 +++++++++----- .../android/wifitrackerlib/StandardWifiEntry.java | 33 ++++++++++++++-------- 3 files changed, 45 insertions(+), 21 deletions(-) (limited to 'libs') diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java index 9aaf346d7..b3b99e0b9 100644 --- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java +++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/OsuWifiEntry.java @@ -35,6 +35,7 @@ import android.os.Handler; import android.text.TextUtils; import android.util.Pair; +import androidx.annotation.GuardedBy; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.WorkerThread; @@ -50,6 +51,9 @@ import java.util.Map; class OsuWifiEntry extends WifiEntry { static final String KEY_PREFIX = "OsuWifiEntry:"; + private final Object mLock = new Object(); + // Scan result list must be thread safe for generating the verbose scan summary + @GuardedBy("mLock") @NonNull private final List mCurrentScanResults = new ArrayList<>(); @NonNull private final String mKey; @@ -265,10 +269,12 @@ class OsuWifiEntry extends WifiEntry { throws IllegalArgumentException { if (scanResults == null) scanResults = new ArrayList<>(); - mCurrentScanResults.clear(); - mCurrentScanResults.addAll(scanResults); + synchronized (mLock) { + mCurrentScanResults.clear(); + mCurrentScanResults.addAll(scanResults); + } - final ScanResult bestScanResult = getBestScanResultByLevel(mCurrentScanResults); + final ScanResult bestScanResult = getBestScanResultByLevel(scanResults); if (bestScanResult == null) { mLevel = WIFI_LEVEL_UNREACHABLE; } else { diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java index b26d035da..e7ed15362 100644 --- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java +++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/PasspointWifiEntry.java @@ -40,6 +40,7 @@ import android.net.wifi.hotspot2.PasspointConfiguration; import android.os.Handler; import android.text.TextUtils; +import androidx.annotation.GuardedBy; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.WorkerThread; @@ -57,7 +58,11 @@ import java.util.StringJoiner; public class PasspointWifiEntry extends WifiEntry { static final String KEY_PREFIX = "PasspointWifiEntry:"; + private final Object mLock = new Object(); + // Scan result list must be thread safe for generating the verbose scan summary + @GuardedBy("mLock") private final List mCurrentHomeScanResults = new ArrayList<>(); + @GuardedBy("mLock") private final List mCurrentRoamingScanResults = new ArrayList<>(); @NonNull private final String mKey; @@ -405,13 +410,15 @@ public class PasspointWifiEntry extends WifiEntry { throws IllegalArgumentException { mIsRoaming = false; mWifiConfig = wifiConfig; - mCurrentHomeScanResults.clear(); - mCurrentRoamingScanResults.clear(); - if (homeScanResults != null) { - mCurrentHomeScanResults.addAll(homeScanResults); - } - if (roamingScanResults != null) { - mCurrentRoamingScanResults.addAll(roamingScanResults); + synchronized (mLock) { + mCurrentHomeScanResults.clear(); + mCurrentRoamingScanResults.clear(); + if (homeScanResults != null) { + mCurrentHomeScanResults.addAll(homeScanResults); + } + if (roamingScanResults != null) { + mCurrentRoamingScanResults.addAll(roamingScanResults); + } } if (mWifiConfig != null) { mSecurity = getSecurityTypeFromWifiConfiguration(wifiConfig); diff --git a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java index 642313a2b..fcf127721 100644 --- a/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java +++ b/libs/WifiTrackerLib/src/com/android/wifitrackerlib/StandardWifiEntry.java @@ -46,6 +46,7 @@ import android.os.Handler; import android.os.SystemClock; import android.text.TextUtils; +import androidx.annotation.GuardedBy; import androidx.annotation.IntDef; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -98,7 +99,10 @@ public class StandardWifiEntry extends WifiEntry { private static final int PSK_WPA_WPA2 = 2; private static final int PSK_UNKNOWN = 3; - private final List mCurrentScanResults = new ArrayList<>(); + private final Object mLock = new Object(); + // Scan result list must be thread safe for generating the verbose scan summary + @GuardedBy("mLock") + @NonNull private final List mCurrentScanResults = new ArrayList<>(); @NonNull private final String mKey; @NonNull private final String mSsid; @@ -616,10 +620,12 @@ public class StandardWifiEntry extends WifiEntry { } } - mCurrentScanResults.clear(); - mCurrentScanResults.addAll(scanResults); + synchronized (mLock) { + mCurrentScanResults.clear(); + mCurrentScanResults.addAll(scanResults); + } - final ScanResult bestScanResult = getBestScanResultByLevel(mCurrentScanResults); + final ScanResult bestScanResult = getBestScanResultByLevel(scanResults); if (bestScanResult == null) { mLevel = WIFI_LEVEL_UNREACHABLE; } else { @@ -731,8 +737,10 @@ public class StandardWifiEntry extends WifiEntry { @Override String getScanResultDescription() { - if (mCurrentScanResults.size() == 0) { - return ""; + synchronized (mLock) { + if (mCurrentScanResults.size() == 0) { + return ""; + } } final StringBuilder description = new StringBuilder(); @@ -745,11 +753,14 @@ public class StandardWifiEntry extends WifiEntry { } private String getScanResultDescription(int minFrequency, int maxFrequency) { - final List scanResults = mCurrentScanResults.stream() - .filter(scanResult -> scanResult.frequency >= minFrequency - && scanResult.frequency <= maxFrequency) - .sorted(Comparator.comparingInt(scanResult -> -1 * scanResult.level)) - .collect(Collectors.toList()); + final List scanResults; + synchronized (mLock) { + scanResults = mCurrentScanResults.stream() + .filter(scanResult -> scanResult.frequency >= minFrequency + && scanResult.frequency <= maxFrequency) + .sorted(Comparator.comparingInt(scanResult -> -1 * scanResult.level)) + .collect(Collectors.toList()); + } final int scanResultCount = scanResults.size(); if (scanResultCount == 0) { -- cgit v1.2.3