From adfe68f282f36328addee80381c7db7a2974fbda Mon Sep 17 00:00:00 2001
From: Patrick Rohr <prohr@google.com>
Date: Tue, 25 Feb 2025 08:47:53 -0800
Subject: [PATCH] Get rid of volatile mIsRunning

The check for mIsRunning inside catch(IOException) was always race-y as
BluetoothServerSocket could have thrown an unrelated exception at the
same time tearDown() is called -- the only side effect being
inconsistent logging. However, it is safe to call
postDestroyAndUnregisterReservedOffer() as that function first checks
whether the offer still exists.

Therefore, this CL just gets rid of mIsRunning in favor of always
calling postDestroyAndUnregisterReservedOffer().

Test: TH
Change-Id: I6a2b48aa54dfa1ad55030bdcef847a1a9efa0787
---
 .../com/android/server/L2capNetworkProvider.java  | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/service/src/com/android/server/L2capNetworkProvider.java b/service/src/com/android/server/L2capNetworkProvider.java
index 2cec820359..f6ca26b1d9 100644
--- a/service/src/com/android/server/L2capNetworkProvider.java
+++ b/service/src/com/android/server/L2capNetworkProvider.java
@@ -271,7 +271,6 @@ public class L2capNetworkProvider {
         private class AcceptThread extends Thread {
             private static final int TIMEOUT_MS = 500;
             private final BluetoothServerSocket mServerSocket;
-            private volatile boolean mIsRunning = true;
 
             public AcceptThread(BluetoothServerSocket serverSocket) {
                 super("L2capNetworkProvider-AcceptThread");
@@ -295,16 +294,17 @@ public class L2capNetworkProvider {
 
             @Override
             public void run() {
-                while (mIsRunning) {
+                while (true) {
                     final BluetoothSocket connectedSocket;
                     try {
                         connectedSocket = mServerSocket.accept();
                     } catch (IOException e) {
-                        // BluetoothServerSocket was closed().
-                        if (!mIsRunning) return;
-
-                        // Else, BluetoothServerSocket encountered exception.
-                        Log.e(TAG, "BluetoothServerSocket#accept failed", e);
+                        // Note calling BluetoothServerSocket#close() also triggers an IOException
+                        // which is indistinguishable from any other exceptional behavior.
+                        // postDestroyAndUnregisterReservedOffer() is always safe to call as it
+                        // first checks whether the offer still exists; so if the
+                        // BluetoothServerSocket was closed (i.e. on tearDown()) this is a noop.
+                        Log.w(TAG, "BluetoothServerSocket closed or #accept failed", e);
                         postDestroyAndUnregisterReservedOffer();
                         return; // stop running immediately on error
                     }
@@ -314,7 +314,6 @@ public class L2capNetworkProvider {
 
             public void tearDown() {
                 HandlerUtils.ensureRunningOnHandlerThread(mHandler);
-                mIsRunning = false;
                 try {
                     // BluetoothServerSocket.close() is thread-safe.
                     mServerSocket.close();