Run the account's loop on its own thread so iCloud calls stop queueing behind fetches

AGENTS.md

@@ -694,6 +694,38 @@ already sent you, and then reasoning from a guess while you wait.
   had a phone number blacked out by hand, and the pixels either side of it were not. Quote the
   error text, never the surrounding screen, and never re-upload the image.
 
+### 21. The account takes turns. It is never used by two things at once
+
+The signed-in account's asyncio loop runs on a thread of its own (`app/src/main/python/account_loop.py`),
+so the iCloud screen no longer waits behind a location fetch. Before
+[#126](https://github.com/parawanderer/OpenTagViewer/issues/126) every call into Python held
+`PythonLock`, and a first import held it for minutes inside a single `fetch_location`.
+
+**That is interleaving, not concurrency, and it must stay that way.** FindMy.py's async account is
+not safe to use from two operations at once. `request_pet`, which opening a keychain session calls,
+swaps the account's login state out for a whole GSA round trip and restores it in a `finally`; a
+report request running in that window reads a `dsid` that is not there. A 401 inside
+`fetch_raw_reports` re-authenticates the same way. So every operation takes the account's turn:
+an ordinary call for its whole length, a location fetch inside `account_loop.interleaving()` once
+per request to Apple. An iCloud step waits for one request, not for the walk.
+
+Three things follow:
+
+- **Every account `main` hands Java is attached** (`account_loop.attach`, straight after it is built
+  and before its first call - the loop has to be idle to be handed over). `icloud_bridge.openSession`
+  refuses an account that is not, because without the thread the two flows would drive one loop
+  with `run_until_complete` at once - the failure the Java lock existed for.
+- **Only location fetches go inside `interleaving()`.** They are known to touch the account in
+  `fetch_raw_reports` alone, which is wrapped to take the turn. Anything else in that block runs
+  with no exclusion at all.
+- **Location fetches still take `PythonLock` against each other.** Two side by side would double the
+  request rate for the same keys (rule 6). The iCloud service is the only caller that dropped it.
+
+`test_account_loop.py` runs against the real pinned `AppleAccount` rather than a fake, because the
+whole arrangement leans on two private details of it - `_evt_loop.run_until_complete` and the
+fetcher looking `fetch_raw_reports` up on the instance - and a fake keeps both names after a rename
+(rule 3).
+
 ---
 
 ## Building and testing

app/src/androidTest/java/dev/wander/android/opentagviewer/FetchFromICloudBackPressTest.java

@@ -245,10 +245,11 @@ public void leavingClosesTheSession() {
      * the in-app arrow and the device's own back button alike - on the reasoning that there is
      * nothing to go back to mid-call. That reasoning holds for an unlock and for nothing else.
      *
-     * <p>What it produced in practice: every call into Python is serialised through
+     * <p>What it produced in practice: every call into Python was serialised through
      * {@code PythonLock}, because the iCloud flow and the location fetch drive the same asyncio
-     * loop, and a first import holds that lock for <b>minutes</b> walking key indices for tags
-     * with no alignment record. Opening this screen during that wait left the user on a spinner
+     * loop, and a first import held that lock for <b>minutes</b> walking key indices for tags
+     * with no alignment record. Issue #126 shortened that wait to one request to Apple, which is
+     * still a call genuinely in flight - so leaving has to keep working. Opening this screen during that wait left the user on a spinner
      * they could not leave by any means, reading "Looking for a device that can unlock it" -
      * which was not what was happening and, for an account already linked, never would be.
      *

app/src/androidTest/java/dev/wander/android/opentagviewer/python/icloud/OpeningARealICloudSessionTest.java

@@ -4,6 +4,7 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
 
 import com.chaquo.python.Kwarg;
 import com.chaquo.python.PyObject;
@@ -108,19 +109,22 @@ public void anaccountTheBridgeCannotUseYieldsNullRatherThanACrash() {
      *
      * <p>The account is assembled here rather than mocked at the Java seam, which is the whole
      * point: {@code openSession} looks for the two private attributes FindMy.py's account carries,
-     * and a {@code SimpleNamespace} with those is enough to get a real {@code ICloudSession} back
-     * across the real bridge. No Apple account, no network, and no fake standing where the bug was.
+     * and a {@code SimpleNamespace} with those - attached, as {@code main} attaches every account -
+     * is enough to get a real {@code ICloudSession} back across the real bridge. No Apple account, no network, and no fake standing where the bug was.
      */
     @Test
     public void asessionThatOpensIsHandedBackRatherThanLostInConversion() {
         final Python python = Python.getInstance();
 
-        // What `openSession` guards on: an async account and a loop to drive it. Nothing is
-        // called on either here - opening a session only stores them.
+        // What `openSession` guards on: an async account and a loop to drive it, with the loop
+        // on its own thread as `main` leaves every account (issue #126). Nothing is called on
+        // either here - opening a session only stores them.
         final PyObject account = python.getModule("types").callAttr(
                 "SimpleNamespace",
                 new Kwarg("_asyncacc", python.getModule("types").callAttr("SimpleNamespace")),
                 new Kwarg("_evt_loop", python.getModule("asyncio").callAttr("new_event_loop")));
+        assertTrue("the account's loop could not be given its own thread",
+                python.getModule("account_loop").callAttr("attach", account).toBoolean());
 
         assertNotNull("a session that opened must reach Java, not be lost converting it",
                 PythonICloudService.openFor(new PythonAppleAccount(account)));

app/src/debug/python/icloud_test_double.py

@@ -27,6 +27,7 @@
 from types import SimpleNamespace
 from typing import Any
 
+import account_loop
 from exporter import icloud
 
 _originals: dict[str, Any] = {}
@@ -245,13 +246,25 @@ def anAccount() -> Any:
     asserting it reached the join cannot pass against code that never read the session at all:
     that path falls back to `identity.LEGACY_SERIAL`, and the two must not be the same string.
     """
-    return SimpleNamespace(
-        _asyncacc=SimpleNamespace(
-            serial=SESSION_SERIAL,
-            identity=SimpleNamespace(
-                model="iPhone17,1", os_name="iPhone OS", os_version="18.1",
-                os_build="22B83", cfnetwork="1568.100.1", darwin="24.1.0")),
-        _evt_loop=asyncio.new_event_loop())
+    account = _Account()
+    account._asyncacc = SimpleNamespace(
+        serial=SESSION_SERIAL,
+        identity=SimpleNamespace(
+            model="iPhone17,1", os_name="iPhone OS", os_version="18.1",
+            os_build="22B83", cfnetwork="1568.100.1", darwin="24.1.0"))
+    account._evt_loop = asyncio.new_event_loop()
+
+    # As `main` does for every account it hands Java, or `openSession` refuses it.
+    account_loop.attach(account)
+
+    return account
+
+
+class _Account:
+    """A plain class rather than a SimpleNamespace, so `attach` can watch it for collection."""
+
+    _asyncacc: Any
+    _evt_loop: Any
 
 
 def whatReachedTheAccount() -> dict[str, Any]:

app/src/main/java/dev/wander/android/opentagviewer/FetchFromICloudActivity.java

@@ -49,7 +49,6 @@
 import dev.wander.android.opentagviewer.python.icloud.ICloudFetch;
 import dev.wander.android.opentagviewer.python.icloud.ICloudService;
 import dev.wander.android.opentagviewer.python.icloud.KeychainMembership;
-import dev.wander.android.opentagviewer.python.PythonLock;
 import dev.wander.android.opentagviewer.python.icloud.RecoverableDevice;
 import dev.wander.android.opentagviewer.ui.CodeChipSpan;
 import dev.wander.android.opentagviewer.ui.RecoverableDeviceIcon;
@@ -230,9 +229,9 @@ private void onBackWithin() {
         if (this.isShowing(R.id.icloud_loading_container)) {
             // **Only an unlock is worth trapping somebody for.** Swallowing every back press
             // while the loading step was up meant that a screen waiting on PythonLock - which a
-            // first import holds for minutes, walking key indices for tags with no alignment -
-            // could not be left at all. Neither the in-app arrow nor the device's own back
-            // button did anything, and the wait looked like a hang.
+            // first import held for minutes, walking key indices for tags with no alignment,
+            // until issue #126 - could not be left at all. Neither the in-app arrow nor the
+            // device's own back button did anything, and the wait looked like a hang.
             //
             // An unlock attempt is different: it is already talking to Apple, and attempts are
             // probably a limited resource on their end, so abandoning one mid-flight risks
@@ -282,12 +281,14 @@ private void start() {
         // by the membership check further down: an app that has already joined resumes as the
         // member it is and never sees the device list. Saying otherwise up front tells somebody
         // who just linked their account that the app is hunting for hardware to unlock with,
-        // when the thing it is actually waiting for is its turn to talk to Python.
+        // when what it is actually doing is opening the account.
+        //
+        // That is also all it is doing while a location check runs. This used to say it was
+        // waiting for one, because it was - for minutes, on a first import - until the two
+        // started taking turns between requests to Apple (issue #126).
         //
         // askForADevice() sets that message, at the point it becomes true.
-        this.showWaiting(PythonLock.isBusy()
-                ? R.string.icloud_loading_waiting_for_location_check
-                : R.string.icloud_loading_opening_account);
+        this.showWaiting(R.string.icloud_loading_opening_account);
 
         if (this.icloud != null) {
             this.icloud.close();

app/src/main/java/dev/wander/android/opentagviewer/python/PythonAppleService.java

@@ -92,10 +92,9 @@ public static PythonAppleService getInstance() {
     }
 
     /**
-     * Whether a call into Python is currently in progress.
+     * Whether a fetch or an account change is currently in progress.
      *
-     * <p>Delegates to {@link PythonLock}, which is where the lock moved when the iCloud flow
-     * started driving the same event loop. Kept here because the periodic refresh asks this
+     * <p>Delegates to {@link PythonLock}. Kept here because the periodic refresh asks this
      * service, and where the lock lives is not its business.
      */
     public static boolean isBusy() {

app/src/main/java/dev/wander/android/opentagviewer/python/PythonLock.java

@@ -4,21 +4,22 @@
 import java.util.concurrent.locks.ReentrantLock;
 
 /**
- * The one lock that every call into Python must be made under.
+ * One location fetch, or one change to which account is signed in, at a time.
  *
- * <p>FindMy.py's synchronous {@code AppleAccount} wraps an async one and drives it with a single
- * asyncio event loop. RxJava schedules our work on a thread pool, and the periodic refresh in
- * {@code MapsActivity} fires every 60 seconds regardless of whether the previous fetch has
- * finished. Two threads calling {@code run_until_complete} on the same loop fails with
- * <i>"RuntimeError: This event loop is already running"</i> and then keeps failing.
+ * <p>RxJava schedules our work on a thread pool, and the periodic refresh in {@code MapsActivity}
+ * fires every 60 seconds regardless of whether the previous fetch has finished. A fetch that
+ * takes longer than the refresh interval is entirely normal for an accessory with no alignment
+ * yet, so two fetches overlapping is not a rare race. Running them side by side would double the
+ * request rate against Apple for the same keys, which is the account-flagging risk rule 6 is
+ * about, and have both write back alignment for the same tags.
  *
- * <p>A fetch that takes longer than the refresh interval is entirely normal for an accessory with
- * no alignment yet, so this is not a rare race.
- *
- * <p><b>It lives here, rather than inside one service, because it is not one service's lock.</b>
- * The iCloud flow drives the <i>same</i> event loop - deliberately, since a second account would
- * be a second device to Apple - so a location fetch and a keychain unlock collide exactly as two
- * location fetches would. A second private lock in a second class would be no lock at all.
+ * <p><b>It is no longer what keeps the asyncio loop from being driven twice at once.</b> It used
+ * to be: FindMy.py's synchronous account entered its event loop per call, and two threads in
+ * {@code run_until_complete} failed with <i>"This event loop is already running"</i>. The loop now
+ * runs on its own thread and the account is shared out in turns on the Python side - see
+ * {@code account_loop.py}. That is why the iCloud flow does not take this lock any more: it used
+ * to wait for a whole fetch, which on a first import was minutes (issue #126), and now waits for
+ * one request at most.
  *
  * <p>Serialising alone is not enough: the periodic refresh would still queue up behind a slow
  * fetch, one entry per minute, and then fire the whole stale backlog at once when it finally
@@ -32,13 +33,12 @@ private PythonLock() {
     }
 
     /**
-     * Run one call into Python, with nothing else in there at the same time.
+     * Run one call into Python, with no other fetch or account change in there at the same time.
      *
      * <p><b>Scope it to the call and nothing more.</b> The reason this is a method taking the
-     * work, rather than a lock to take and release, is the iCloud flow: it is several calls with
-     * a person answering a dialog between them, and holding this across one of those waits would
-     * stop every location refresh in the app until they got round to typing. Each step takes it,
-     * finishes, and gives it back; the waiting happens in Java with nothing held.
+     * work, rather than a lock to take and release, is that anything held across a wait for a
+     * person - a sign-in code, a passcode dialog - would stop every location refresh in the app
+     * until they got round to typing.
      */
     public static <T> T holding(final Callable<T> work) throws Exception {
         LOCK.lock();
@@ -50,7 +50,7 @@ public static <T> T holding(final Callable<T> work) throws Exception {
     }
 
     /**
-     * Whether a call into Python is currently in progress.
+     * Whether a fetch or an account change is currently in progress.
      *
      * <p>Advisory only. A caller that acts on this can still be beaten to the lock, which is
      * harmless: it just waits, exactly as it did before.

app/src/main/java/dev/wander/android/opentagviewer/python/icloud/PythonICloudService.java

@@ -12,7 +12,6 @@
 import java.util.List;
 
 import dev.wander.android.opentagviewer.python.PythonAppleAccount;
-import dev.wander.android.opentagviewer.python.PythonLock;
 import io.reactivex.rxjava3.core.Completable;
 import io.reactivex.rxjava3.core.Observable;
 import io.reactivex.rxjava3.schedulers.Schedulers;
@@ -195,10 +194,7 @@ public Observable<List<AccessoryRecords>> records(final List<String> beaconIds)
     @Override
     public void close() {
         try {
-            PythonLock.holding(() -> {
-                this.session.callAttr("close");
-                return null;
-            });
+            this.session.callAttr("close");
         } catch (Exception e) {
             // Callers close from a `finally`. Throwing here would replace whatever real failure
             // sent them there with a confusing one about closing.
@@ -211,20 +207,20 @@ public void close() {
      *
      * <p>Two things happen here and both matter.
      *
-     * <p><b>The Python lock is taken for the call and given straight back.</b> The iCloud flow and
-     * the app's periodic location refresh drive the same asyncio event loop, and two threads in
-     * {@code run_until_complete} on one loop fails permanently. Scoping it to the call rather
-     * than the flow is what keeps a passcode dialog from freezing every location update in the
-     * app until the user types.
+     * <p><b>No {@code PythonLock} here, and that is the point of issue #126.</b> The iCloud flow
+     * and the location fetch share one account and one asyncio loop, and taking the Java lock
+     * made this wait for a whole fetch - minutes, on a first import. The loop now runs on a thread
+     * of its own and the account is shared out in turns on the Python side, a fetch giving way
+     * between requests to Apple, so a step here waits for one request at most. See
+     * {@code account_loop.py}.
      *
      * <p><b>A reported failure becomes an exception; a raised one is not expected.</b> The bridge
      * returns failures as values precisely so their message is never empty, so anything actually
      * thrown here is a bug rather than a user-facing condition - and is reported as
      * {@link ICloudFailure#UNKNOWN} with whatever text it had.
      */
     private JSONObject answered(final String step, final Object... arguments) throws Exception {
-        final String returned = PythonLock.holding(
-                () -> this.session.callAttr(step, arguments).toString());
+        final String returned = this.session.callAttr(step, arguments).toString();
 
         final JSONObject answer = new JSONObject(returned);