CAT via Bluetooyh is broken again

[patrickrb]

Closes #781

What changed

Fixes a check-then-act (TOCTOU) race in the Bluetooth CAT write path that
crashed the CAT worker mid-QSO and left the link dead with no recovery.

Symptom (issue #781): on Android 8 and 13/14 the rig either fails to
connect at all, or connects and drops after ~10 s with no reconnect. Last
known good version is dev.1026. The USB-serial path was already hardened
against the analogous race (CableSerialPort.writeIfOpen, PR #651), but the
Bluetooth twin never received the fix.

Root cause: a background INTENT_ACTION_DISCONNECT broadcast (rig
powered off, RFCOMM link dropped) runs disconnect() on another thread,
which nulls the socket field. A CAT/TX worker already past the
connected check in BluetoothSerialService.write() /
BluetoothSerialSocket.write() then dereferences the now-null socket
(socket.write / socket.getOutputStream) — NullPointerException. That
NPE escapes BluetoothRigConnector.sendCommand's IOException-only catch
and kills the CAT worker with no recovery.

Fix (mirrors CableSerialPort.writeIfOpen):

• Mark socket and connected volatile in both
  BluetoothSerialSocket and BluetoothSerialService, so
  connect / disconnect / read-loop / CAT-TX threads see consistent values
  under the Java memory model.
• In write(), snapshot the volatile socket ONCE into a local, then route
  it through a new pure helper
  BluetoothSerialSocket.writeIfConnected(connected, sink, data). A
  concurrent disconnect() that nulls the field after the snapshot can no
  longer flip the write into an NPE — the helper reports the torn-down
  link as the "not connected" IOException sendCommand already handles.

How to test

• Unit: cd ft8af && ./gradlew testDebugUnitTest --tests com.k1af.ft8af.bluetooth.BluetoothSerialWriteTest
  (JDK 17 on macOS; cmd.exe /c "gradlew.bat testDebugUnitTest --tests ..." on Windows).
  Four pure-JVM cases cover: the race case (connected==true but
  snapshot -> sink is null must throw IOException("not connected"), not
  NPE), an already-disconnected link, a successful write, and IOException
  propagation from the sink.
• Full suite: ./gradlew testDebugUnitTest — passes locally.
• Manual on device: pair a BT CAT rig (e.g. Yaesu with SCU-58), start a
  QSO, then power the rig off during TX. Expected: app logs "not
  connected" and keeps running instead of the CAT thread dying; on
  repower + reconnect, CAT resumes.

[Copilot]

🟢 Approval recommended

The functional fix is localized and covered by new unit tests; remaining feedback is limited to minor documentation/test determinism nits.

Pull request overview

This PR fixes a TOCTOU (check-then-act) race in the CAT-over-Bluetooth write path where a concurrent disconnect could null the socket between a connection check and the actual write, leading to an uncaught NullPointerException and a dead CAT worker thread.

Changes:

• Make the Bluetooth socket and connection state volatile to improve cross-thread visibility.
• Snapshot the socket once in write() and route the write through a new BluetoothSerialSocket.writeIfConnected(...) helper to convert the race into a recoverable IOException("not connected").
• Add a pure-JVM unit test suite covering the race case, disconnected case, success case, and IOException propagation.

File summaries

File	Description
ft8af/app/src/main/java/com/k1af/ft8af/bluetooth/BluetoothSerialSocket.java	Adds volatile state + socket snapshotting and a shared writeIfConnected guard to prevent NPEs during concurrent disconnects.
ft8af/app/src/main/java/com/k1af/ft8af/bluetooth/BluetoothSerialService.java	Snapshots the service socket once and uses the shared guard to avoid check-then-act races in the service layer.
ft8af/app/src/test/java/com/k1af/ft8af/bluetooth/BluetoothSerialWriteTest.java	Adds pure-JVM tests that validate the guard behavior and the exact race condition.

Review details

Suppressed comments (1)

ft8af/app/src/main/java/com/k1af/ft8af/bluetooth/BluetoothSerialSocket.java:129

• writeIfConnected Javadoc says disconnect runs before connected was cleared, but connected isn't cleared in disconnect(); it is cleared later in the read-loop exception path. Updating the wording avoids implying an ordering guarantee that doesn't exist.

     * call. When the link is down ({@code !connected}) or the snapshot was
     * already null (disconnect() ran first, before {@code connected} was cleared),
     * throw the {@code "not connected"} {@link IOException} the CAT connector

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.