Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9574a1be authored by Quang Luong's avatar Quang Luong
Browse files

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
parent 1f66726d

packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java

+66 −39
Original line number Diff line number Diff line
@@ -55,6 +55,7 @@ import android.util.ArraySet; 
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 @@ public class AccessPoint implements Comparable<AccessPoint> { 
    /** 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 @@ public class AccessPoint implements Comparable<AccessPoint> { 
    }



    /** 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,9 +500,11 @@ public class AccessPoint implements Comparable<AccessPoint> { 


        if (isVerboseLoggingEnabled()) {

            builder.append(",rssi=").append(mRssi);

            synchronized (mLock) {

                builder.append(",scan cache size=").append(mScanResults.size()

                        + mExtraScanResults.size());

            }

        }



        return builder.append(')').toString();

    }
@@ -532,6 +545,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
     */

    private boolean updateScores(WifiNetworkScoreCache scoreCache, long maxScoreCacheAgeMillis) {

        long nowMillis = SystemClock.elapsedRealtime();

        synchronized (mLock) {

            for (ScanResult result : mScanResults) {

                ScoredNetwork score = scoreCache.getScoredNetwork(result);

                if (score == null) {
@@ -546,6 +560,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
                    timedScore.update(score, nowMillis);

                }

            }

        }



        // Remove old cached networks

        long evictionCutoff = nowMillis - maxScoreCacheAgeMillis;
@@ -619,6 +634,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
                mIsScoredNetworkMetered |= score.meteredHint;

            }

        } else {

            synchronized (mLock) {

                for (ScanResult result : mScanResults) {

                    ScoredNetwork score = scoreCache.getScoredNetwork(result);

                    if (score == null) {
@@ -627,6 +643,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
                    mIsScoredNetworkMetered |= score.meteredHint;

                }

            }

        }

        return oldMetering == mIsScoredNetworkMetered;

    }
@@ -741,8 +758,10 @@ public class AccessPoint implements Comparable<AccessPoint> { 
     */

    public Set<ScanResult> getScanResults() {

        Set<ScanResult> allScans = new ArraySet<>();

        synchronized (mLock) {

            allScans.addAll(mScanResults);

            allScans.addAll(mExtraScanResults);

        }

        return allScans;

    }
@@ -766,12 +785,14 @@ public class AccessPoint implements Comparable<AccessPoint> { 


        ScanResult bestResult = null;

        int bestRssi = UNREACHABLE_RSSI;

        synchronized (mLock) {

            for (ScanResult result : mScanResults) {

                if (result.level > bestRssi) {

                    bestRssi = result.level;

                    bestResult = result;

                }

            }

        }



        // Set the rssi to the average of the current rssi and the previous rssi.

        if (bestRssi != UNREACHABLE_RSSI && mRssi != UNREACHABLE_RSSI) {
@@ -1211,9 +1232,11 @@ public class AccessPoint implements Comparable<AccessPoint> { 
        savedState.putInt(KEY_EAPTYPE, mEapType);

        if (mConfig != null) savedState.putParcelable(KEY_CONFIG, mConfig);

        savedState.putParcelable(KEY_WIFIINFO, mInfo);

        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 @@ public class AccessPoint implements Comparable<AccessPoint> { 
        }



        int oldLevel = getLevel();

        synchronized (mLock) {

            mScanResults.clear();

            mScanResults.addAll(scanResults);

        }

        updateBestRssiInfo();

        int newLevel = getLevel();
@@ -1324,18 +1349,20 @@ public class AccessPoint implements Comparable<AccessPoint> { 
    void setScanResultsPasspoint(

            @Nullable Collection<ScanResult> homeScans,

            @Nullable Collection<ScanResult> roamingScans) {

        synchronized (mLock) {

            mExtraScanResults.clear();

            if (!CollectionUtils.isEmpty(homeScans)) {

                mIsRoaming = false;

                if (!CollectionUtils.isEmpty(roamingScans)) {

                    mExtraScanResults.addAll(roamingScans);

                }

            mIsRoaming = false;

                setScanResults(homeScans);

            } else if (!CollectionUtils.isEmpty(roamingScans)) {

                mIsRoaming = true;

                setScanResults(roamingScans);

            }

        }

    }



    /**

     * Attempt to update the AccessPoint with the current connection info.