What it produced in practice: every call into Python is serialised through + *
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 minutes 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 minutes 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. * diff --git a/app/src/androidTest/java/dev/wander/android/opentagviewer/python/icloud/OpeningARealICloudSessionTest.java b/app/src/androidTest/java/dev/wander/android/opentagviewer/python/icloud/OpeningARealICloudSessionTest.java index 745c372b..ba65958e 100644 --- a/app/src/androidTest/java/dev/wander/android/opentagviewer/python/icloud/OpeningARealICloudSessionTest.java +++ b/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() { * *
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))); diff --git a/app/src/debug/python/icloud_test_double.py b/app/src/debug/python/icloud_test_double.py index 400e76ce..531aba0f 100644 --- a/app/src/debug/python/icloud_test_double.py +++ b/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]: diff --git a/app/src/main/java/dev/wander/android/opentagviewer/FetchFromICloudActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/FetchFromICloudActivity.java index a4bf3945..7f931343 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/FetchFromICloudActivity.java +++ b/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(); diff --git a/app/src/main/java/dev/wander/android/opentagviewer/python/PythonAppleService.java b/app/src/main/java/dev/wander/android/opentagviewer/python/PythonAppleService.java index 6ce38090..7ba8565e 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/python/PythonAppleService.java +++ b/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. * - *
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 + *
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() { diff --git a/app/src/main/java/dev/wander/android/opentagviewer/python/PythonLock.java b/app/src/main/java/dev/wander/android/opentagviewer/python/PythonLock.java index cda507bf..a76ec215 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/python/PythonLock.java +++ b/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. * - *
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 - * "RuntimeError: This event loop is already running" and then keeps failing. + *
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. * - *
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. - * - *
It lives here, rather than inside one service, because it is not one service's lock. - * The iCloud flow drives the same 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. + *
It is no longer what keeps the asyncio loop from being driven twice at once. 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 "This event loop is already running". 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. * *
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. * *
Scope it to the call and nothing more. 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 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.
diff --git a/app/src/main/java/dev/wander/android/opentagviewer/python/icloud/PythonICloudService.java b/app/src/main/java/dev/wander/android/opentagviewer/python/icloud/PythonICloudService.java
index 243e726d..2a28fd98 100644
--- a/app/src/main/java/dev/wander/android/opentagviewer/python/icloud/PythonICloudService.java
+++ b/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 Two things happen here and both matter.
*
- * The Python lock is taken for the call and given straight back. 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.
+ * No {@code PythonLock} here, and that is the point of issue #126. 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}.
*
* A reported failure becomes an exception; a raised one is not expected. The bridge
* returns failures as values precisely so their message is never empty, so anything actually
@@ -223,8 +220,7 @@ public void close() {
* {@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);
diff --git a/app/src/main/python/account_loop.py b/app/src/main/python/account_loop.py
new file mode 100644
index 00000000..17ee0262
--- /dev/null
+++ b/app/src/main/python/account_loop.py
@@ -0,0 +1,328 @@
+"""
+The account's event loop, run on a thread of its own instead of being borrowed by each caller.
+
+FindMy.py's synchronous `AppleAccount` drives its async account by calling
+`self._evt_loop.run_until_complete(...)` from whichever thread asked. A loop can only be run by
+one thread at a time, so Java serialised every call into Python behind `PythonLock` - and a
+first import, which walks thousands of key indices inside a *single* `fetch_location`, held it
+for minutes. The iCloud screen waited for the whole walk before it could say anything to Apple
+(issue #126).
+
+:func:`attach` swaps the account's `_evt_loop` for an :class:`AccountLoop`, whose
+`run_until_complete` hands the coroutine to the real loop - now running forever on its own
+thread - and waits for the answer. **Same loop, same aiohttp session, same identity**: rule 11 is
+untouched, because only *who drives* the loop changes, not what is on it.
+
+**The account is still used by one thing at a time, and that is deliberate.** FindMy.py's async
+account is not safe to use concurrently, and not only in theory: `request_pet`, which opening a
+keychain session calls, swaps the account's login state out for the length of a GSA round trip
+and puts it back in a `finally`. Anything reading `dsid` or `service_tokens` in that window gets
+a `KeyError` or an `InvalidStateError`. A 401 inside `fetch_raw_reports` re-authenticates the
+same way. So every operation takes a turn on the account, and what changed is the size of a
+turn:
+
+- an ordinary call takes one turn for its whole length - the exclusion it always had, measured
+ in coroutines rather than Java calls
+- a location fetch inside :func:`interleaving` takes a turn **per request to Apple** instead,
+ by way of a wrapped `fetch_raw_reports`, and nothing between requests touches the account
+
+So a call that arrives during a long walk waits for one request, not for the walk. Between
+requests the fetch is only deriving keys and decrypting, which needs nothing the account holds.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import concurrent.futures
+import contextvars
+import threading
+import traceback
+import weakref
+from contextlib import contextmanager
+from typing import Any, Awaitable, Callable, Coroutine, Iterator, TypeVar
+
+_T = TypeVar("_T")
+
+# How long closing waits for cancelled calls to unwind, and then for the thread to stop. Both
+# are about a coroutine that will not let go, which should never happen; the bound is there so
+# that it cannot hang whoever is closing the account.
+_SHUTDOWN_TIMEOUT_SECONDS = 10.0
+
+# Set on the calling thread by `interleaving`, read by `run_until_complete` on the same thread.
+_interleaving = threading.local()
+
+# Whether the running task already holds the account's turn. A context variable rather than a
+# task identity, so that it is inherited by anything the holder spawns - which is part of the same
+# operation, and would otherwise deadlock waiting for a turn its own parent is holding.
+_holdingTurn: contextvars.ContextVar[bool] = contextvars.ContextVar(
+ "holdingTurn", default=False)
+
+
+@contextmanager
+def interleaving() -> Iterator[None]:
+ """
+ Let the account calls made on this thread take turns per request rather than per call.
+
+ For location fetches, and only for them: they are the long calls, and the only ones whose
+ requests are known to be separable - `fetch_raw_reports` is the one place they touch the
+ account. Anything else made inside this block would run without any exclusion at all.
+ """
+ previous = getattr(_interleaving, "on", False)
+ _interleaving.on = True
+ try:
+ yield
+ finally:
+ _interleaving.on = previous
+
+
+class AccountLoop:
+ """
+ Stands in for an account's `_evt_loop`, which FindMy.py uses for nothing but
+ `run_until_complete`.
+
+ Deliberately not a general proxy for the loop. Anything that reached past these methods for
+ `run_forever` or `stop` would be fighting the thread that owns it, so they are not offered.
+ """
+
+ def __init__(self, loop: asyncio.AbstractEventLoop) -> None:
+ self._loop = loop
+ self._turn = asyncio.Lock()
+ # Guards `_closed` against a submission racing a close: anything submitted before the
+ # flag flips is on the loop by the time the close cancels what is there.
+ self._state = threading.Lock()
+ self._closed = False
+ # Whether report requests take turns of their own. Until they do, a fetch inside
+ # `interleaving` must take one turn for its whole length like anything else, or it would
+ # run with no exclusion at all.
+ self._turnsPerRequest = False
+ self._thread = threading.Thread(
+ target=self._runForever, name="findmy-account-loop", daemon=True)
+ self._thread.start()
+
+ def _runForever(self) -> None:
+ asyncio.set_event_loop(self._loop)
+ self._loop.run_forever()
+
+ @property
+ def loop(self) -> asyncio.AbstractEventLoop:
+ return self._loop
+
+ def is_running(self) -> bool:
+ return not self._closed and self._loop.is_running()
+
+ def is_closed(self) -> bool:
+ return self._closed or self._loop.is_closed()
+
+ def run_until_complete(self, coro: Coroutine[Any, Any, _T]) -> _T:
+ """What FindMy.py's sync wrapper calls: run this on the account's loop, in its turn."""
+ try:
+ if self._turnsPerRequest and getattr(_interleaving, "on", False):
+ return self._submit(coro)
+
+ return self._submit(self.inTurn(lambda: coro))
+ finally:
+ # A no-op once it has run. For one refused, or cancelled while still queued for the
+ # turn, it is what stops "coroutine was never awaited" landing in the log as if it
+ # were a bug.
+ coro.close()
+
+ async def inTurn(self, makeCall: Callable[[], Awaitable[_T]]) -> _T:
+ """
+ Make and await a call while holding the account's turn, or straight through if this
+ task already holds it.
+
+ **A factory, so the call is made inside the turn.** FindMy.py checks the login state
+ when an account method is *called*, not when it is awaited - and `request_pet` holding
+ the turn is exactly when that state is temporarily wrong.
+
+ **Reentrant on purpose.** An ordinary call that reaches `fetch_raw_reports` - a sync
+ `fetch_location` made outside `interleaving` - holds the turn already, and an
+ `asyncio.Lock` taken twice by one holder waits on itself forever.
+ """
+ if _holdingTurn.get():
+ return await makeCall()
+
+ async with self._turn:
+ holding = _holdingTurn.set(True)
+ try:
+ return await makeCall()
+ finally:
+ _holdingTurn.reset(holding)
+
+ def _submit(self, coro: Coroutine[Any, Any, _T]) -> _T:
+ if threading.current_thread() is self._thread:
+ # Blocking the loop's own thread on something only that thread can run never ends.
+ # An exception is a far better way to meet this than a frozen app.
+ coro.close()
+ raise RuntimeError(
+ "A synchronous account call was made from the account's own event loop thread")
+
+ with self._state:
+ if self._closed:
+ coro.close()
+ raise RuntimeError("This account has been closed")
+
+ future = asyncio.run_coroutine_threadsafe(coro, self._loop)
+
+ try:
+ return future.result()
+ except concurrent.futures.CancelledError:
+ # Only `close` cancels anything here, so this is the account going away under a call
+ # that was waiting for it - a reason worth more than a bare CancelledError.
+ raise RuntimeError(
+ "The account was closed while this call was waiting for it") from None
+
+ def stopOnceIdle(self) -> None:
+ """
+ Stop the thread once whatever is already on the loop has finished, without waiting here.
+
+ For an account dropped without being closed - a failed sign-in, a code screen somebody
+ walked away from - which used to cost two sockets until the collector ran, and would now
+ also cost a thread idling forever. Called from a finaliser, on whatever thread the
+ collector happens to be on, so it must not block.
+
+ **Finished, not cancelled.** FindMy.py's own finaliser has just scheduled the session's
+ `close` on this loop, and cancelling it would leak the very sockets it is closing.
+ """
+ with self._state:
+ if self._closed:
+ return
+ self._closed = True
+
+ try:
+ self._loop.call_soon_threadsafe(self._loop.create_task, _finishThenStop())
+ except RuntimeError:
+ # The loop is closed already, so there is no thread left to stop.
+ pass
+
+ def shutDown(self) -> bool:
+ """
+ Cancel whatever is still waiting, stop the loop, and stop its thread.
+
+ :return: whether the thread stopped, which is what makes the loop safe to hand back.
+ """
+ with self._state:
+ if self._closed:
+ return not self._thread.is_alive()
+ self._closed = True
+
+ try:
+ asyncio.run_coroutine_threadsafe(
+ _cancelEverythingElse(), self._loop).result(_SHUTDOWN_TIMEOUT_SECONDS)
+ except Exception:
+ print(f"Account loop: calls still waiting did not unwind:\n{traceback.format_exc()}")
+
+ self._loop.call_soon_threadsafe(self._loop.stop)
+ self._thread.join(_SHUTDOWN_TIMEOUT_SECONDS)
+
+ if self._thread.is_alive():
+ print("Account loop: its thread did not stop; leaving the loop where it is.")
+ return False
+
+ return True
+
+
+async def _cancelEverythingElse() -> None:
+ current = asyncio.current_task()
+ others = [task for task in asyncio.all_tasks() if task is not current]
+
+ for task in others:
+ task.cancel()
+
+ await asyncio.gather(*others, return_exceptions=True)
+
+
+async def _finishThenStop() -> None:
+ current = asyncio.current_task()
+ others = [task for task in asyncio.all_tasks() if task is not current]
+
+ await asyncio.gather(*others, return_exceptions=True)
+ asyncio.get_running_loop().stop()
+
+
+def attach(account: Any) -> bool:
+ """
+ Run this account's loop on a thread of its own, from now until :func:`detach`.
+
+ Call it once, straight after the account is built and before anything else uses it: the
+ loop has to be idle to be handed to another thread.
+
+ :return: whether it is attached. False leaves the account as FindMy.py built it, driven by
+ whoever calls it - and :func:`icloud_bridge.openSession` refuses such an account, because
+ the iCloud flow is only safe beside a location fetch when this is in place.
+ """
+ current = getattr(account, "_evt_loop", None)
+ asyncAccount = getattr(account, "_asyncacc", None)
+
+ if isinstance(current, AccountLoop):
+ return True
+
+ if current is None or asyncAccount is None:
+ print(
+ "FindMy.py's account internals have changed: no _evt_loop or _asyncacc, so the"
+ " account's loop cannot be given a thread of its own.")
+ return False
+
+ if current.is_running() or current.is_closed():
+ print("Not attaching the account loop: it is already running somewhere, or closed.")
+ return False
+
+ attached = AccountLoop(current)
+ _takeTurnsPerRequest(asyncAccount, attached)
+ account._evt_loop = attached
+
+ try:
+ # Holds the loop, never the account, or the account could never be collected to fire it.
+ weakref.finalize(account, attached.stopOnceIdle)
+ except TypeError:
+ # Only an object that cannot be weakly referenced, which FindMy.py's account can be.
+ # The thread then lives until `detach`, which is a leak and not a fault.
+ print("Account loop: this account cannot be watched for collection; close it explicitly.")
+
+ return True
+
+
+def _takeTurnsPerRequest(asyncAccount: Any, attached: AccountLoop) -> None:
+ """
+ Make each report request take the account's turn on its own.
+
+ `fetch_raw_reports` is the one place a location fetch touches the account: it reads the
+ service token, asks Anisette for headers, and on a 401 re-authenticates. Every walk through
+ key indices in FindMy.py's report fetcher comes through here, one request at a time, so this
+ is the boundary where something else can be let in.
+
+ Set on the instance, which is where the fetcher looks it up (`self._account.fetch_raw_reports`).
+
+ **Missing is survivable, so it is not an error.** Attaching still gives the loop its thread,
+ and fetches then take one turn each, exactly as they did before this module. Raising instead
+ would fail the restore, and a failed restore signs the user out.
+ """
+ original = getattr(asyncAccount, "fetch_raw_reports", None)
+
+ if original is None:
+ print(
+ "FindMy.py's account has no fetch_raw_reports, so location fetches cannot give way"
+ " between requests; each will hold the account until it finishes.")
+ return
+
+ async def fetch_raw_reports(*args: Any, **kwargs: Any) -> Any:
+ return await attached.inTurn(lambda: original(*args, **kwargs))
+
+ asyncAccount.fetch_raw_reports = fetch_raw_reports
+ attached._turnsPerRequest = True
+
+
+def detach(account: Any) -> None:
+ """
+ Stop the account's loop thread and hand the loop back, once the account is closed.
+
+ Handed back rather than left behind so a closed account behaves exactly as it did before
+ this module existed - including `Closable.__del__`, which closes on whatever loop it finds.
+ """
+ attached = getattr(account, "_evt_loop", None)
+
+ if not isinstance(attached, AccountLoop):
+ return
+
+ if attached.shutDown():
+ account._evt_loop = attached.loop
diff --git a/app/src/main/python/icloud_bridge.py b/app/src/main/python/icloud_bridge.py
index 2b85beb8..f47b67b8 100644
--- a/app/src/main/python/icloud_bridge.py
+++ b/app/src/main/python/icloud_bridge.py
@@ -42,6 +42,7 @@
import traceback
from typing import Any
+import account_loop
import identity as app_identity
from exporter import icloud
from exporter.identity import written_by_opentagviewer
@@ -344,9 +345,10 @@ class ICloudSession:
:meth:`close` when the screen is finished with - in a `finally`, because two of these steps
hold sockets and an abandoned session leaks them for the life of the process.
- **Not thread-safe, and it cannot be made so.** Every step runs a coroutine on the account's
- own event loop, and a loop cannot be driven from two threads at once. Java calls these from
- a single Rx chain.
+ **Not thread-safe with itself.** The steps build on each other's state, and Java calls them
+ from a single Rx chain. Beside a location fetch it is safe, and does not wait for one to
+ finish: the account's loop runs on its own thread and each step takes its turn on the
+ account between the fetch's requests - see :mod:`account_loop`.
"""
def __init__(self, account: Any, asyncAccount: Any, loop: Any) -> None:
@@ -911,4 +913,13 @@ def openSession(account: Any) -> ICloudSession | None:
" iCloud flow cannot be driven from the account the app is signed in with.")
return None
+ if not isinstance(loop, account_loop.AccountLoop):
+ # Java no longer makes these steps wait behind a location fetch, which is only safe
+ # with the loop on its own thread. Without it, both would run `run_until_complete` on
+ # one loop at once - "This event loop is already running", for good.
+ print(
+ "The account's loop is not on a thread of its own, so the iCloud flow cannot run"
+ " beside a location fetch. See account_loop.attach.")
+ return None
+
return ICloudSession(account, asyncAccount, loop)
diff --git a/app/src/main/python/main.py b/app/src/main/python/main.py
index 5bab609f..0431c81a 100644
--- a/app/src/main/python/main.py
+++ b/app/src/main/python/main.py
@@ -30,6 +30,7 @@
SyncSecondFactorMethod
)
+import account_loop
import identity as app_identity
@@ -318,6 +319,8 @@ def loginSync(email: str, password: str, anisetteServerUrl: str,
timeout=LOGIN_TIMEOUT_SECONDS,
**app_identity.deviceIdsForNewSession(localAnisette),
)
+ # Before the first call, while the loop is still idle - see account_loop.attach.
+ account_loop.attach(acc)
state = acc.login(email, password)
@@ -839,6 +842,7 @@ def getAccount(
acc = AppleAccount.from_json(data)
_preferLocalAnisette(acc, localAnisette)
+ account_loop.attach(acc)
print(f"Restored account, login state: {acc.login_state}")
@@ -975,6 +979,9 @@ def closeAccount(account: AppleAccount) -> bool:
loop.run_until_complete(account.close())
print("Closed the account's HTTP session and connector.")
+ # After the close, which takes its turn like any other call: the loop's thread is only
+ # stopped once nothing on it still needs the session.
+ account_loop.detach(account)
return True
except Exception:
# Discarding this account regardless, so a failure here changes nothing the caller does.
@@ -1959,7 +1966,10 @@ def getLastReports(
# which meant no beacon's updated alignment was persisted - so every later
# fetch started from the same wide range again and never converged.
try:
- fetched = _fetchReportsForAccessory(account, airtag, start_dt, now_dt)
+ # Turns per request rather than per fetch, so the iCloud screen is not kept
+ # waiting for a whole key walk - see account_loop.
+ with account_loop.interleaving():
+ fetched = _fetchReportsForAccessory(account, airtag, start_dt, now_dt)
reports = fetched.reports or []
except Exception:
failures += 1
@@ -2097,7 +2107,8 @@ def getReports(
width_before = _isAlignmentWide(airtag, start_dt, end_dt)
try:
- reports = _fetchReportsInRange(account, airtag, start_dt, end_dt) or []
+ with account_loop.interleaving():
+ reports = _fetchReportsInRange(account, airtag, start_dt, end_dt) or []
except Exception:
failures += 1
print(f"Fetch failed for {beaconId}, continuing with the rest: "
diff --git a/app/src/main/res/values-de/strings.xml b/app/src/main/res/values-de/strings.xml
index 6bda6f93..0406b6eb 100644
--- a/app/src/main/res/values-de/strings.xml
+++ b/app/src/main/res/values-de/strings.xml
@@ -247,7 +247,6 @@ Du kannst sie später wieder verknüpfen. Dafür brauchst du den Gerätecode dei