joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Update VcnNetworkProvider to use thread safe Sets

Browse source 
This patch is to fix a race condition when two threads
access VcnNetworkProvider.mRequests at the same time. The
issue happened when the VCN thread is removing elements and the
binder thread is iterating the Set. It causes the binder thread
to end up with a Null element and throw NPE

This patch fixes this threading issue by changing the Set to
be a thread-safe Set

Bug: 382106220
Test: atest CtsVcnTestCases && atest FrameworksVcnTests
Test: Set up test environment to make the race condition 90%
      reproducible; verified the fix by running the test for
      100 times on forrest
Flag: EXEMPT; low risk and small scope bug fix
Change-Id: Id7598a861368454efcd0aa49721c54e11aaebdf1
This commit is contained in:
Yan Yan 2025-01-09 11:11:56 -08:00
commit ce63e16ca8
2 changed files with 7 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Vcn/service-b/src/com/android/server/vcn/Vcn.java
View file

@ -209,6 +209,8 @@ public class Vcn extends Handler {
this(vcnContext, subscriptionGroup, config, snapshot, vcnCallback, new Dependencies());
}
// WARNING: This constructor executes on the binder thread. Thread safety MUST be ensured when
// accessing data within this constructor and any methods called from here.
@VisibleForTesting(visibility = Visibility.PRIVATE)
public Vcn(
@NonNull VcnContext vcnContext,

8
Vcn/service-b/src/com/android/server/vcn/VcnNetworkProvider.java
View file

@ -36,7 +36,6 @@ import android.net.vcn.VcnGatewayConnectionConfig;
import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.util.ArraySet;
import android.util.IndentingPrintWriter;
import android.util.Slog;
@ -46,6 +45,7 @@ import com.android.modules.utils.HandlerExecutor;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executor;
/**
@ -56,12 +56,14 @@ import java.util.concurrent.Executor;
*
* @hide
*/
// TODO(b/388919146): Implement a more generic solution to prevent concurrent modifications on
// mListeners and mRequests
// TODO(b/374174952): Replace VANILLA_ICE_CREAM with BAKLAVA after Android B finalization
@TargetApi(Build.VERSION_CODES.VANILLA_ICE_CREAM)
public class VcnNetworkProvider extends NetworkProvider {
private static final String TAG = VcnNetworkProvider.class.getSimpleName();
private final Set<NetworkRequestListener> mListeners = new ArraySet<>();
private final Set<NetworkRequestListener> mListeners = ConcurrentHashMap.newKeySet();
private final Context mContext;
private final Handler mHandler;
@ -72,7 +74,7 @@ public class VcnNetworkProvider extends NetworkProvider {
*
* <p>NetworkRequests are immutable once created, and therefore can be used as stable keys.
*/
private final Set<NetworkRequest> mRequests = new ArraySet<>();
private final Set<NetworkRequest> mRequests = ConcurrentHashMap.newKeySet();
public VcnNetworkProvider(@NonNull Context context, @NonNull Looper looper) {
this(context, looper, new Dependencies());