BT Classic/BTBR: Fix concurrency issues on connection

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/AbstractHeadphoneDeviceSupport.java

@@ -25,7 +25,6 @@ import nodomain.freeyourgadget.gadgetbridge.model.NotificationSpec;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.AbstractSerialDeviceSupport;
 
 public abstract class AbstractHeadphoneDeviceSupport extends AbstractSerialDeviceSupport implements HeadphoneHelper.Callback {
-
     private HeadphoneHelper headphoneHelper;
 
     @Override
@@ -44,12 +43,6 @@ public abstract class AbstractHeadphoneDeviceSupport extends AbstractSerialDevic
         headphoneHelper = new HeadphoneHelper(getContext(), getDevice(), this);
     }
 
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public void onSendConfiguration(String config) {
         if (!headphoneHelper.onSendConfiguration(config))
@@ -58,8 +51,10 @@ public abstract class AbstractHeadphoneDeviceSupport extends AbstractSerialDevic
 
     @Override
     public void dispose() {
-        if (headphoneHelper != null)
+        synchronized (ConnectionMonitor) {
-            headphoneHelper.dispose();
+            if (headphoneHelper != null)
-        super.dispose();
+                headphoneHelper.dispose();
+            super.dispose();
+        }
     }
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/btbr/AbstractBTBRDeviceSupport.java

@@ -36,6 +36,10 @@ import nodomain.freeyourgadget.gadgetbridge.service.AbstractDeviceSupport;
  * @see nodomain.freeyourgadget.gadgetbridge.service.btclassic.BtClassicIoThread
  */
 public abstract class AbstractBTBRDeviceSupport extends AbstractDeviceSupport implements SocketCallback {
+
+    /// used to guard {@link #connect()}, {@link #disconnect()} and {@link #dispose()}
+    protected final Object ConnectionMonitor = new Object();
+
     private BtBRQueue mQueue;
     private UUID mSupportedService = null;
     private int mBufferSize = 1024;
@@ -50,20 +54,24 @@ public abstract class AbstractBTBRDeviceSupport extends AbstractDeviceSupport im
 
     @Override
     public boolean connect() {
-        final UUID supportedService = getSupportedService();
+        synchronized (ConnectionMonitor) {
-        if (supportedService == null) {
+            final UUID supportedService = getSupportedService();
-            throw new NullPointerException("No supported service UUID specified");
+            if (supportedService == null) {
-        }
+                throw new NullPointerException("No supported service UUID specified");
+            }
 
-        if (mQueue == null) {
+            if (mQueue == null) {
-            mQueue = new BtBRQueue(getBluetoothAdapter(), getDevice(), getContext(), this, supportedService, getBufferSize());
+                mQueue = new BtBRQueue(getBluetoothAdapter(), getDevice(), getContext(), this, supportedService, getBufferSize());
+            }
+            return mQueue.connect();
         }
-        return mQueue.connect();
     }
 
     public void disconnect() {
-        if (mQueue != null) {
+        synchronized (ConnectionMonitor) {
-            mQueue.disconnect();
+            if (mQueue != null) {
+                mQueue.disconnect();
+            }
         }
     }
 
@@ -80,9 +88,11 @@ public abstract class AbstractBTBRDeviceSupport extends AbstractDeviceSupport im
 
     @Override
     public void dispose() {
-        if (mQueue != null) {
+        synchronized (ConnectionMonitor) {
-            mQueue.dispose();
+            if (mQueue != null) {
-            mQueue = null;
+                mQueue.dispose();
+                mQueue = null;
+            }
         }
     }
 
@@ -91,7 +101,7 @@ public abstract class AbstractBTBRDeviceSupport extends AbstractDeviceSupport im
     }
 
     @Override
-    public boolean isConnected(){
+    public boolean isConnected() {
         // in a multi-threaded environment the queue knows
         // best about the up-to-date connection status
         return (mQueue != null) && mQueue.isConnected();

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/btbr/BtBRQueue.java

@@ -33,7 +33,6 @@ import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Locale;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
@@ -171,13 +170,11 @@ public final class BtBRQueue {
                                 return;
                             }
 
-                            if (!(msg.obj instanceof Transaction)) {
+                            if (!(msg.obj instanceof Transaction transaction)) {
                                 LOG.error("msg.obj is not an instance of Transaction");
                                 return;
                             }
 
-                            Transaction transaction = (Transaction) msg.obj;
-
                             for (BtBRAction action : transaction.getActions()) {
                                 if (LOG.isDebugEnabled()) {
                                     LOG.debug("About to run action: {}", action);
@@ -212,8 +209,15 @@ public final class BtBRQueue {
      */
     @SuppressLint("MissingPermission")
     public boolean connect() {
-        if (isConnected()) {
+        final GBDevice.State state = mGbDevice.getState();
-            LOG.warn("Ignoring connect() because already connected.");
+        if (state.equalsOrHigherThan(GBDevice.State.CONNECTING)) {
+            LOG.warn("connect - ignored, state is {}", state);
+            return false;
+        } else if (mBtSocket != null) {
+            LOG.warn("connect - ignored, mBtSocket isn't null");
+            return false;
+        } else if (mDisposed.get()) {
+            LOG.error("connect - ignored, this BtBRQueue has already been disposed");
             return false;
         }
 
@@ -304,6 +308,7 @@ public final class BtBRQueue {
 
     public void dispose() {
         if (mDisposed.getAndSet(true)) {
+            LOG.warn("dispose() was called repeatedly");
             return;
         }
 

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/aawireless/AAWirelessSupport.java

@@ -101,13 +101,15 @@ public class AAWirelessSupport extends AbstractBTBRDeviceSupport {
 
     @Override
     public void dispose() {
-        try {
+        synchronized (ConnectionMonitor) {
-            getContext().unregisterReceiver(commandReceiver);
+            try {
-        } catch (final Exception e) {
+                getContext().unregisterReceiver(commandReceiver);
-            LOG.warn("Failed to unregister receiver", e);
+            } catch (final Exception e) {
-        }
+                LOG.warn("Failed to unregister receiver", e);
+            }
 
-        super.dispose();
+            super.dispose();
+        }
     }
 
     @Override

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/divoom/PixooSupport.java

@@ -19,28 +19,16 @@ package nodomain.freeyourgadget.gadgetbridge.service.devices.divoom;
 
 import android.net.Uri;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import nodomain.freeyourgadget.gadgetbridge.service.serial.AbstractSerialDeviceSupport;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceIoThread;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceProtocol;
 
 public class PixooSupport extends AbstractSerialDeviceSupport {
-    private static final Logger LOG = LoggerFactory.getLogger(PixooSupport.class);
-
-
     @Override
     public void onSendConfiguration(String config) {
         super.onSendConfiguration(config);
     }
 
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public synchronized PixooIOThread getDeviceIOThread() {
         return (PixooIOThread) super.getDeviceIOThread();

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/huami/zeppos/ZeppOsBtbrSupport.java

@@ -117,9 +117,11 @@ public class ZeppOsBtbrSupport extends AbstractBTBRDeviceSupport implements Zepp
 
     @Override
     public void dispose() {
-        zeppOsSupport.dispose();
+        synchronized (ConnectionMonitor) {
-        pingHandler.removeCallbacksAndMessages(null);
+            zeppOsSupport.dispose();
-        super.dispose();
+            pingHandler.removeCallbacksAndMessages(null);
+            super.dispose();
+        }
     }
 
     @Override

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/huawei/HuaweiBRSupport.java

@@ -199,8 +199,10 @@ public class HuaweiBRSupport extends AbstractBTBRDeviceSupport {
 
     @Override
     public void dispose() {
-        supportProvider.dispose();
+        synchronized (ConnectionMonitor) {
-        super.dispose();
+            supportProvider.dispose();
+            super.dispose();
+        }
     }
 
     @Override

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/huawei/HuaweiFreebudsSupport.java

@@ -108,9 +108,11 @@ public class HuaweiFreebudsSupport extends HuaweiBRSupport implements HeadphoneH
 
     @Override
     public void dispose() {
-        if (headphoneHelper != null)
+        synchronized (ConnectionMonitor) {
-            headphoneHelper.dispose();
+            if (headphoneHelper != null)
-        super.dispose();
+                headphoneHelper.dispose();
+            super.dispose();
+        }
     }
 
     @Override

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/liveview/LiveviewSupport.java

@@ -17,29 +17,11 @@
     along with this program.  If not, see <https://www.gnu.org/licenses/>. */
 package nodomain.freeyourgadget.gadgetbridge.service.devices.liveview;
 
-import android.net.Uri;
-
-import java.util.ArrayList;
-import java.util.UUID;
-
-import nodomain.freeyourgadget.gadgetbridge.model.Alarm;
-import nodomain.freeyourgadget.gadgetbridge.model.CalendarEventSpec;
-import nodomain.freeyourgadget.gadgetbridge.model.CallSpec;
-import nodomain.freeyourgadget.gadgetbridge.model.MusicSpec;
-import nodomain.freeyourgadget.gadgetbridge.model.MusicStateSpec;
-import nodomain.freeyourgadget.gadgetbridge.model.NotificationSpec;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.AbstractSerialDeviceSupport;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceIoThread;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceProtocol;
 
 public class LiveviewSupport extends AbstractSerialDeviceSupport {
-
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     protected GBDeviceProtocol createDeviceProtocol() {
         return new LiveviewProtocol(getDevice());
@@ -59,9 +41,4 @@ public class LiveviewSupport extends AbstractSerialDeviceSupport {
     public synchronized LiveviewIoThread getDeviceIOThread() {
         return (LiveviewIoThread) super.getDeviceIOThread();
     }
-
-    @Override
-    public void onNotification(NotificationSpec notificationSpec) {
-        super.onNotification(notificationSpec);
-    }
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleSupport.java

@@ -55,8 +55,10 @@ public class PebbleSupport extends AbstractSerialDeviceSupport {
 
     @Override
     public void dispose() {
-        super.dispose();
+        synchronized (ConnectionMonitor) {
-        unregisterSunriseSunsetAlarmReceiver();
+            super.dispose();
+            unregisterSunriseSunsetAlarmReceiver();
+        }
     }
 
     private void registerSunriseSunsetAlarmReceiver() {
@@ -79,8 +81,13 @@ public class PebbleSupport extends AbstractSerialDeviceSupport {
 
     @Override
     public boolean connect() {
-        getDeviceIOThread().start();
+        synchronized (ConnectionMonitor) {
-        registerSunriseSunsetAlarmReceiver();
+            final PebbleIoThread deviceIOThread = getDeviceIOThread();
+            if (!deviceIOThread.isAlive()) {
+                deviceIOThread.start();
+            }
+            registerSunriseSunsetAlarmReceiver();
+        }
         return true;
     }
 

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pixel/PixelBudsADeviceSupport.java

@@ -5,13 +5,6 @@ import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceIoThread;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceProtocol;
 
 public class PixelBudsADeviceSupport extends AbstractSerialDeviceSupport {
-
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public boolean useAutoConnect() {
         return false;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/qc35/QC35BaseSupport.java

@@ -21,14 +21,6 @@ import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceIoThread;
 import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceProtocol;
 
 public class QC35BaseSupport extends AbstractSerialDeviceSupport {
-
-    @Override
-    public boolean connect() {
-        getDeviceProtocol();
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public boolean useAutoConnect() {
         return false;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/redmibuds/RedmiBudsDeviceSupport.java

@@ -33,12 +33,6 @@ public class RedmiBudsDeviceSupport extends AbstractSerialDeviceSupport {
                 RedmiBudsDeviceSupport.this, getBluetoothAdapter());
     }
 
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public boolean useAutoConnect() {
         return false;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/roidmi/RoidmiSupport.java

@@ -79,11 +79,14 @@ public class RoidmiSupport extends AbstractSerialDeviceSupport {
 
     @Override
     public boolean connect() {
-        getDeviceIOThread().start();
+        synchronized (ConnectionMonitor) {
-
+            final RoidmiIoThread deviceIOThread = getDeviceIOThread();
-        requestDeviceInfos(1500);
+            if (!deviceIOThread.isAlive()) {
-
+                deviceIOThread.start();
-        return true;
+                requestDeviceInfos(1500);
+            }
+            return true;
+        }
     }
 
     @Override
@@ -104,7 +107,7 @@ public class RoidmiSupport extends AbstractSerialDeviceSupport {
 
     @Override
     public void onSendConfiguration(final String config) {
-        LOG.debug("onSendConfiguration " + config);
+        LOG.debug("onSendConfiguration {}", config);
 
         final RoidmiIoThread roidmiIoThread = getDeviceIOThread();
         final RoidmiProtocol roidmiProtocol = (RoidmiProtocol) getDeviceProtocol();
@@ -120,7 +123,7 @@ public class RoidmiSupport extends AbstractSerialDeviceSupport {
                 roidmiIoThread.write(roidmiProtocol.encodeGetVoltage());
                 break;
             default:
-                LOG.error("Invalid Roidmi configuration " + config);
+                LOG.error("Invalid Roidmi configuration {}", config);
                 break;
         }
     }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/soundcore/liberty/SoundcoreLiberty3ProDeviceSupport.java

@@ -9,12 +9,6 @@ import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceProtocol;
 public class SoundcoreLiberty3ProDeviceSupport extends AbstractSerialDeviceSupport {
     public static final UUID UUID_DEVICE_CTRL = UUID.fromString("0cf12d31-fac3-4553-bd80-d6832e7b3952");
 
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public boolean useAutoConnect() {
         return false;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/soundcore/liberty/SoundcoreLiberty4NCDeviceSupport.java

@@ -9,12 +9,6 @@ import nodomain.freeyourgadget.gadgetbridge.service.serial.GBDeviceProtocol;
 public class SoundcoreLiberty4NCDeviceSupport extends AbstractSerialDeviceSupport {
     public static final UUID UUID_DEVICE_CTRL = UUID.fromString("0cf12d31-fac3-4553-bd80-d6832e7b3947");
 
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public boolean useAutoConnect() {
         return false;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/soundcore/q30/SoundcoreQ30DeviceSupport.java

@@ -16,12 +16,6 @@ public class SoundcoreQ30DeviceSupport extends AbstractSerialDeviceSupport {
         return new SoundcoreQ30IOThread(getDevice(), getContext(), (SoundcoreQ30Protocol) getDeviceProtocol(),this, getBluetoothAdapter());
     }
 
-    @Override
-    public boolean connect() {
-        getDeviceIOThread().start();
-        return true;
-    }
-
     @Override
     public boolean useAutoConnect() {
         return false;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/serial/AbstractSerialDeviceSupport.java

@@ -56,6 +56,9 @@ import nodomain.freeyourgadget.gadgetbridge.service.AbstractDeviceSupport;
 public abstract class AbstractSerialDeviceSupport extends AbstractDeviceSupport {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractSerialDeviceSupport.class);
 
+    /// used to guard {@link #connect()} and {@link #dispose()}
+    protected final Object ConnectionMonitor = new Object();
+
     private GBDeviceProtocol gbDeviceProtocol;
     protected GBDeviceIoThread gbDeviceIOThread;
 
@@ -69,13 +72,26 @@ public abstract class AbstractSerialDeviceSupport extends AbstractDeviceSupport
      */
     protected abstract GBDeviceIoThread createDeviceIOThread();
 
+    @Override
+    public boolean connect() {
+        synchronized (ConnectionMonitor) {
+            final GBDeviceIoThread deviceIOThread = getDeviceIOThread();
+            if (!deviceIOThread.isAlive()) {
+                deviceIOThread.start();
+            }
+            return true;
+        }
+    }
+
     @Override
     public void dispose() {
-        // currently only one thread allowed
+        synchronized (ConnectionMonitor) {
-        if (gbDeviceIOThread != null) {
+            // currently only one thread allowed
-            gbDeviceIOThread.quit();
+            if (gbDeviceIOThread != null) {
-            gbDeviceIOThread.interrupt();
+                gbDeviceIOThread.quit();
-            gbDeviceIOThread = null;
+                gbDeviceIOThread.interrupt();
+                gbDeviceIOThread = null;
+            }
         }
     }