From 2195bb65b0a876af4e4748b821f4ac17c5816d23 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Wed, 19 Feb 2025 13:16:38 -0800 Subject: [PATCH 1/2] Add thread checks This CL adds thread checks to various functions. It also adds 2 missing @Override annotations to make it clear that run() is part of the Thread itself. Test: TH Change-Id: Ic89a85b25bdf6afaab0310a641956001d7c83a77 --- .../android/server/L2capNetworkProvider.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/service/src/com/android/server/L2capNetworkProvider.java b/service/src/com/android/server/L2capNetworkProvider.java index 72bd858fe5..76d76a6477 100644 --- a/service/src/com/android/server/L2capNetworkProvider.java +++ b/service/src/com/android/server/L2capNetworkProvider.java @@ -58,6 +58,7 @@ import android.util.ArraySet; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; +import com.android.net.module.util.HandlerUtils; import com.android.net.module.util.ServiceConnectivityJni; import com.android.server.net.L2capNetwork; @@ -226,6 +227,7 @@ public class L2capNetworkProvider { } private void destroyAndUnregisterReservedOffer(ReservedServerOffer reservedOffer) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); // Ensure the offer still exists if this was posted on the handler. if (!mReservedServerOffers.contains(reservedOffer)) return; mReservedServerOffers.remove(reservedOffer); @@ -237,6 +239,7 @@ public class L2capNetworkProvider { @Nullable private L2capNetwork createL2capNetwork(BluetoothSocket socket, NetworkCapabilities caps, L2capNetwork.ICallback cb) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); final String ifname = TUN_IFNAME + String.valueOf(sTunIndex++); final ParcelFileDescriptor tunFd = mDeps.createTunInterface(ifname); if (tunFd == null) { @@ -273,18 +276,21 @@ public class L2capNetworkProvider { } private void postDestroyAndUnregisterReservedOffer() { + // Called on AcceptThread mHandler.post(() -> { destroyAndUnregisterReservedOffer(ReservedServerOffer.this); }); } private void postCreateServerNetwork(BluetoothSocket connectedSocket) { + // Called on AcceptThread mHandler.post(() -> { final boolean success = createServerNetwork(connectedSocket); if (!success) closeBluetoothSocket(connectedSocket); }); } + @Override public void run() { while (mIsRunning) { final BluetoothSocket connectedSocket; @@ -304,6 +310,7 @@ public class L2capNetworkProvider { } public void tearDown() { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); mIsRunning = false; try { // BluetoothServerSocket.close() is thread-safe. @@ -320,6 +327,7 @@ public class L2capNetworkProvider { } private boolean createServerNetwork(BluetoothSocket socket) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); // It is possible the offer went away. if (!mReservedServerOffers.contains(this)) return false; @@ -332,10 +340,12 @@ public class L2capNetworkProvider { new L2capNetwork.ICallback() { @Override public void onError(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); destroyAndUnregisterReservedOffer(ReservedServerOffer.this); } @Override public void onNetworkUnwanted(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); // Leave reservation in place. final boolean networkExists = mL2capNetworks.remove(network); if (!networkExists) return; // already torn down. @@ -375,6 +385,7 @@ public class L2capNetworkProvider { /** Called when the reservation goes away and the reserved offer must be torn down. */ public void tearDown() { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); mAcceptThread.tearDown(); for (L2capNetwork network : mL2capNetworks) { network.tearDown(); @@ -428,6 +439,7 @@ public class L2capNetworkProvider { mSocket = socket; } + @Override public void run() { try { mSocket.connect(); @@ -447,6 +459,7 @@ public class L2capNetworkProvider { } public void abort() { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); mIsAborted = true; // Closing the BluetoothSocket is the only way to unblock connect() because it calls // shutdown on the underlying (connected) SOCK_SEQPACKET. @@ -462,6 +475,7 @@ public class L2capNetworkProvider { private boolean createClientNetwork(L2capNetworkSpecifier specifier, BluetoothSocket socket) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); // Check whether request still exists final ClientRequestInfo cri = mClientNetworkRequests.get(specifier); if (cri == null) return false; @@ -478,10 +492,12 @@ public class L2capNetworkProvider { // `network` cannot be used directly. @Override public void onError(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); declareAllNetworkRequestsUnfulfillable(specifier); } @Override public void onNetworkUnwanted(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); declareAllNetworkRequestsUnfulfillable(specifier); } }); @@ -595,6 +611,7 @@ public class L2capNetworkProvider { * Only call this when all associated NetworkRequests have been released. */ private void releaseClientNetworkRequest(ClientRequestInfo cri) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); mClientNetworkRequests.remove(cri.specifier); if (cri.connectThread.isAlive()) { // Note that if ConnectThread succeeds between calling #isAlive() and #abort(), the @@ -610,6 +627,7 @@ public class L2capNetworkProvider { } private void declareAllNetworkRequestsUnfulfillable(L2capNetworkSpecifier specifier) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); final ClientRequestInfo cri = mClientNetworkRequests.get(specifier); if (cri == null) return; From ef2e0123b4aa02db0caa1fc041db10e4b3346eb7 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Wed, 19 Feb 2025 16:18:20 -0800 Subject: [PATCH 2/2] Make linter happier Add 4 more spaces. Test: TH Change-Id: I89f65af37f847211ba49876bd7776ab44afbcb00 --- .../android/server/L2capNetworkProvider.java | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/service/src/com/android/server/L2capNetworkProvider.java b/service/src/com/android/server/L2capNetworkProvider.java index 76d76a6477..67813358b8 100644 --- a/service/src/com/android/server/L2capNetworkProvider.java +++ b/service/src/com/android/server/L2capNetworkProvider.java @@ -338,19 +338,19 @@ public class L2capNetworkProvider { final L2capNetwork network = createL2capNetwork(socket, mReservedCapabilities, new L2capNetwork.ICallback() { - @Override - public void onError(L2capNetwork network) { - HandlerUtils.ensureRunningOnHandlerThread(mHandler); - destroyAndUnregisterReservedOffer(ReservedServerOffer.this); - } - @Override - public void onNetworkUnwanted(L2capNetwork network) { - HandlerUtils.ensureRunningOnHandlerThread(mHandler); - // Leave reservation in place. - final boolean networkExists = mL2capNetworks.remove(network); - if (!networkExists) return; // already torn down. - network.tearDown(); - } + @Override + public void onError(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); + destroyAndUnregisterReservedOffer(ReservedServerOffer.this); + } + @Override + public void onNetworkUnwanted(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); + // Leave reservation in place. + final boolean networkExists = mL2capNetworks.remove(network); + if (!networkExists) return; // already torn down. + network.tearDown(); + } }); if (network == null) { @@ -486,20 +486,20 @@ public class L2capNetworkProvider { final L2capNetwork network = createL2capNetwork(socket, caps, new L2capNetwork.ICallback() { - // TODO: do not send onUnavailable() after the network has become available. The - // right thing to do here is to tearDown the network (if it still exists, because - // note that the request might have already been removed in the meantime, so - // `network` cannot be used directly. - @Override - public void onError(L2capNetwork network) { - HandlerUtils.ensureRunningOnHandlerThread(mHandler); - declareAllNetworkRequestsUnfulfillable(specifier); - } - @Override - public void onNetworkUnwanted(L2capNetwork network) { - HandlerUtils.ensureRunningOnHandlerThread(mHandler); - declareAllNetworkRequestsUnfulfillable(specifier); - } + // TODO: do not send onUnavailable() after the network has become available. The + // right thing to do here is to tearDown the network (if it still exists, + // because note that the request might have already been removed in the + // meantime, so `network` cannot be used directly. + @Override + public void onError(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); + declareAllNetworkRequestsUnfulfillable(specifier); + } + @Override + public void onNetworkUnwanted(L2capNetwork network) { + HandlerUtils.ensureRunningOnHandlerThread(mHandler); + declareAllNetworkRequestsUnfulfillable(specifier); + } }); if (network == null) return false;