Merge changes Ib2febede,Id3752790,I7cfcc93a,I26f6669f,I5d722c0d into main
* changes:
Add missing javadocs to satisfy linter
Make linter happy
Use ThreadLeakMonitor for CSL2capProviderTest
Add some tests to ensure BluetoothExceptions are handled
Move creating L2capPacketForwarder to dependencies
diff --git a/service/src/com/android/server/L2capNetworkProvider.java b/service/src/com/android/server/L2capNetworkProvider.java
index 2ff8565..814a068 100644
--- a/service/src/com/android/server/L2capNetworkProvider.java
+++ b/service/src/com/android/server/L2capNetworkProvider.java
@@ -61,6 +61,7 @@
import com.android.net.module.util.HandlerUtils;
import com.android.net.module.util.ServiceConnectivityJni;
import com.android.server.net.L2capNetwork;
+import com.android.server.net.L2capPacketForwarder;
import java.io.IOException;
import java.util.ArrayList;
@@ -245,7 +246,8 @@
return null;
}
- return L2capNetwork.create(mHandler, mContext, mProvider, ifname, socket, tunFd, caps, cb);
+ return L2capNetwork.create(
+ mHandler, mContext, mProvider, ifname, socket, tunFd, caps, mDeps, cb);
}
private static void closeBluetoothSocket(BluetoothSocket socket) {
@@ -271,6 +273,7 @@
private volatile boolean mIsRunning = true;
public AcceptThread(BluetoothServerSocket serverSocket) {
+ super("L2capNetworkProvider-AcceptThread");
mServerSocket = serverSocket;
}
@@ -434,6 +437,7 @@
private volatile boolean mIsAborted = false;
public ConnectThread(L2capNetworkSpecifier specifier, BluetoothSocket socket) {
+ super("L2capNetworkProvider-ConnectThread");
mSpecifier = specifier;
mSocket = socket;
}
@@ -646,6 +650,7 @@
return thread;
}
+ /** Create a tun interface configured for blocking i/o */
@Nullable
public ParcelFileDescriptor createTunInterface(String ifname) {
final ParcelFileDescriptor fd;
@@ -668,13 +673,19 @@
}
return fd;
}
+
+ /** Create an L2capPacketForwarder and start forwarding */
+ public L2capPacketForwarder createL2capPacketForwarder(Handler handler,
+ ParcelFileDescriptor tunFd, BluetoothSocket socket, boolean compressHeaders,
+ L2capPacketForwarder.ICallback cb) {
+ return new L2capPacketForwarder(handler, tunFd, socket, compressHeaders, cb);
+ }
}
public L2capNetworkProvider(Context context) {
this(new Dependencies(), context);
}
- @VisibleForTesting
public L2capNetworkProvider(Dependencies deps, Context context) {
mDeps = deps;
mContext = context;
diff --git a/service/src/com/android/server/net/L2capNetwork.java b/service/src/com/android/server/net/L2capNetwork.java
index b624bca..c7417f9 100644
--- a/service/src/com/android/server/net/L2capNetwork.java
+++ b/service/src/com/android/server/net/L2capNetwork.java
@@ -38,6 +38,8 @@
import android.os.ParcelFileDescriptor;
import android.util.Log;
+import com.android.server.L2capNetworkProvider;
+
public class L2capNetwork {
private static final NetworkScore NETWORK_SCORE = new NetworkScore.Builder().build();
private final String mLogTag;
@@ -115,7 +117,7 @@
public L2capNetwork(String logTag, Handler handler, Context context, NetworkProvider provider,
BluetoothSocket socket, ParcelFileDescriptor tunFd, NetworkCapabilities nc,
- LinkProperties lp, ICallback cb) {
+ LinkProperties lp, L2capNetworkProvider.Dependencies deps, ICallback cb) {
mLogTag = logTag;
mHandler = handler;
mNetworkCapabilities = nc;
@@ -123,7 +125,8 @@
final L2capNetworkSpecifier spec = (L2capNetworkSpecifier) nc.getNetworkSpecifier();
final boolean compressHeaders = spec.getHeaderCompression() == HEADER_COMPRESSION_6LOWPAN;
- mForwarder = new L2capPacketForwarder(handler, tunFd, socket, compressHeaders, () -> {
+ mForwarder = deps.createL2capPacketForwarder(handler, tunFd, socket, compressHeaders,
+ () -> {
// TODO: add a check that this callback is invoked on the handler thread.
cb.onError(L2capNetwork.this);
});
@@ -146,7 +149,7 @@
@Nullable
public static L2capNetwork create(Handler handler, Context context, NetworkProvider provider,
String ifname, BluetoothSocket socket, ParcelFileDescriptor tunFd,
- NetworkCapabilities nc, ICallback cb) {
+ NetworkCapabilities nc, L2capNetworkProvider.Dependencies deps, ICallback cb) {
// TODO: add a check that this function is invoked on the handler thread.
final String logTag = String.format("L2capNetwork[%s]", ifname);
@@ -157,7 +160,8 @@
final LinkProperties lp = new L2capIpClient(logTag, context, ifname).start();
if (lp == null) return null;
- return new L2capNetwork(logTag, handler, context, provider, socket, tunFd, nc, lp, cb);
+ return new L2capNetwork(
+ logTag, handler, context, provider, socket, tunFd, nc, lp, deps, cb);
}
/** Get the NetworkCapabilities used for this Network */
diff --git a/service/src/com/android/server/net/L2capPacketForwarder.java b/service/src/com/android/server/net/L2capPacketForwarder.java
index 00faecf..737cb9c 100644
--- a/service/src/com/android/server/net/L2capPacketForwarder.java
+++ b/service/src/com/android/server/net/L2capPacketForwarder.java
@@ -233,6 +233,7 @@
L2capThread(IReadWriteFd readFd, IReadWriteFd writeFd, boolean isIngress,
boolean compressHeaders) {
+ super("L2capNetworkProvider-ForwarderThread");
mLogTag = isIngress ? "L2capForwarderThread-Ingress" : "L2capForwarderThread-Egress";
mReadFd = readFd;
mWriteFd = writeFd;
diff --git a/tests/unit/java/com/android/server/connectivityservice/CSL2capProviderTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSL2capProviderTest.kt
index d44bd0a..489c3ad 100644
--- a/tests/unit/java/com/android/server/connectivityservice/CSL2capProviderTest.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/CSL2capProviderTest.kt
@@ -20,42 +20,37 @@
import android.bluetooth.BluetoothManager
import android.bluetooth.BluetoothServerSocket
import android.bluetooth.BluetoothSocket
-import android.content.Context
import android.net.L2capNetworkSpecifier
import android.net.L2capNetworkSpecifier.HEADER_COMPRESSION_6LOWPAN
import android.net.L2capNetworkSpecifier.HEADER_COMPRESSION_NONE
import android.net.L2capNetworkSpecifier.ROLE_SERVER
-import android.net.NetworkCapabilities
import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED
import android.net.NetworkCapabilities.NET_CAPABILITY_TRUSTED
import android.net.NetworkCapabilities.TRANSPORT_BLUETOOTH
-import android.net.NetworkProvider
-import android.net.NetworkProvider.NetworkOfferCallback
import android.net.NetworkRequest
-import android.net.NetworkScore
import android.net.NetworkSpecifier
import android.os.Build
import android.os.HandlerThread
-import android.os.Looper
-import com.android.server.L2capNetworkProvider
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
import com.android.testutils.DevSdkIgnoreRunner
import com.android.testutils.RecorderCallback.CallbackEntry.Reserved
+import com.android.testutils.RecorderCallback.CallbackEntry.Unavailable
import com.android.testutils.TestableNetworkCallback
+import com.android.testutils.waitForIdle
import java.io.IOException
-import java.util.concurrent.Executor
+import java.util.Optional
import java.util.concurrent.LinkedBlockingQueue
import kotlin.test.assertEquals
+import kotlin.test.assertFalse
import kotlin.test.assertNull
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
-import org.mockito.Mock
import org.mockito.Mockito.doAnswer
import org.mockito.Mockito.doReturn
+import org.mockito.Mockito.doThrow
import org.mockito.Mockito.mock
-import org.mockito.MockitoAnnotations
private const val PSM = 0x85
private val REMOTE_MAC = byteArrayOf(1, 2, 3, 4, 5, 6)
@@ -67,14 +62,19 @@
@RunWith(DevSdkIgnoreRunner::class)
@IgnoreUpTo(Build.VERSION_CODES.R)
+@DevSdkIgnoreRunner.MonitorThreadLeak
class CSL2capProviderTest : CSTest() {
private val btAdapter = mock<BluetoothAdapter>()
private val btServerSocket = mock<BluetoothServerSocket>()
private val btSocket = mock<BluetoothSocket>()
private val providerDeps = mock<L2capNetworkProvider.Dependencies>()
- private val acceptQueue = LinkedBlockingQueue<BluetoothSocket>()
+ // BlockingQueue does not support put(null) operations, as null is used as an internal sentinel
+ // value. Therefore, use Optional<BluetoothSocket> where an empty optional signals the
+ // BluetoothServerSocket#close() operation.
+ private val acceptQueue = LinkedBlockingQueue<Optional<BluetoothSocket>>()
private val handlerThread = HandlerThread("CSL2capProviderTest thread").apply { start() }
+ private val registeredCallbacks = ArrayList<TestableNetworkCallback>()
// Requires Dependencies mock to be setup before creation.
private lateinit var provider: L2capNetworkProvider
@@ -83,15 +83,16 @@
fun innerSetUp() {
doReturn(btAdapter).`when`(bluetoothManager).getAdapter()
doReturn(btServerSocket).`when`(btAdapter).listenUsingInsecureL2capChannel()
- doReturn(PSM).`when`(btServerSocket).getPsm();
+ doReturn(PSM).`when`(btServerSocket).getPsm()
doAnswer {
val sock = acceptQueue.take()
- sock ?: throw IOException()
+ if (sock == null || !sock.isPresent()) throw IOException()
+ sock.get()
}.`when`(btServerSocket).accept()
doAnswer {
- acceptQueue.put(null)
+ acceptQueue.put(Optional.empty())
}.`when`(btServerSocket).close()
doReturn(handlerThread).`when`(providerDeps).getHandlerThread()
@@ -101,16 +102,30 @@
@After
fun innerTearDown() {
+ // Unregistering a callback which has previously been unregistered by virtue of receiving
+ // onUnavailable is a noop.
+ registeredCallbacks.forEach { cm.unregisterNetworkCallback(it) }
+ // Wait for CS handler idle, meaning the unregisterNetworkCallback has been processed and
+ // L2capNetworkProvider has been notified.
+ waitForIdle()
+
+ // While quitSafely() effectively waits for idle, it is not enough, because the tear down
+ // path itself posts on the handler thread. This means that waitForIdle() needs to run
+ // twice. The first time, to ensure all active threads have been joined, and the second time
+ // to run all associated clean up actions.
+ handlerThread.waitForIdle(HANDLER_TIMEOUT_MS)
handlerThread.quitSafely()
handlerThread.join()
}
private fun reserveNetwork(nr: NetworkRequest) = TestableNetworkCallback().also {
cm.reserveNetwork(nr, csHandler, it)
+ registeredCallbacks.add(it)
}
private fun requestNetwork(nr: NetworkRequest) = TestableNetworkCallback().also {
cm.requestNetwork(nr, it, csHandler)
+ registeredCallbacks.add(it)
}
private fun NetworkRequest.copyWithSpecifier(specifier: NetworkSpecifier): NetworkRequest {
@@ -188,4 +203,42 @@
nr = REQUEST.copyWithSpecifier(specifier)
reserveNetwork(nr).assertNoCallback()
}
+
+ @Test
+ fun testBluetoothException_listenUsingInsecureL2capChannelThrows() {
+ doThrow(IOException()).`when`(btAdapter).listenUsingInsecureL2capChannel()
+ var specifier = L2capNetworkSpecifier.Builder()
+ .setRole(ROLE_SERVER)
+ .setHeaderCompression(HEADER_COMPRESSION_6LOWPAN)
+ .build()
+ var nr = REQUEST.copyWithSpecifier(specifier)
+ reserveNetwork(nr).expect<Unavailable>()
+
+ doReturn(btServerSocket).`when`(btAdapter).listenUsingInsecureL2capChannel()
+ reserveNetwork(nr).expect<Reserved>()
+ }
+
+ @Test
+ fun testBluetoothException_acceptThrows() {
+ doThrow(IOException()).`when`(btServerSocket).accept()
+ var specifier = L2capNetworkSpecifier.Builder()
+ .setRole(ROLE_SERVER)
+ .setHeaderCompression(HEADER_COMPRESSION_6LOWPAN)
+ .build()
+ var nr = REQUEST.copyWithSpecifier(specifier)
+ val cb = reserveNetwork(nr)
+ cb.expect<Reserved>()
+ cb.expect<Unavailable>()
+
+ // BluetoothServerSocket#close() puts Optional.empty() on the acceptQueue.
+ acceptQueue.clear()
+ doAnswer {
+ val sock = acceptQueue.take()
+ assertFalse(sock.isPresent())
+ throw IOException() // to indicate the socket was closed.
+ }.`when`(btServerSocket).accept()
+ val cb2 = reserveNetwork(nr)
+ cb2.expect<Reserved>()
+ cb2.assertNoCallback()
+ }
}