beeware / toga

A Python native, OS native GUI toolkit.
https://toga.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
4.36k stars 674 forks source link

Android event loop is not thread-safe #2138

Open mhsmith opened 1 year ago

mhsmith commented 1 year ago

In https://github.com/beeware/toga/pull/1706, we made Toga use call_soon_threadsafe when adding tasks from another thread. However, it doesn't look like the current Android implementation is actually thread-safe.

In the base class in asyncio.base_events, the implementations of call_soon and call_soon_threadsafe are virtually identical. Both of them call _call_soon, which is itself thread-safe because all it does is add the task to a deque. The only difference is that call_soon_threadsafe then writes something to a pipe to wake up the event loop.

However, the Android event loop overrides _call_soon to call enqueue_android_wakeup_for_delayed_tasks, which calls _get_next_delayed_task_wakeup, which iterates and modifies many data structures in a way that can't possibly be safe to do outside of the event loop's own thread.

The simplest solution I can think of is to move the call of _get_next_delayed_task_wakeup into run_delayed_tasks, which always runs on the event loop's own thread, and then pass the result as an argument to enqueue_android_wakeup_for_delayed_tasks. In the case of _call_soon, that argument would always be zero.

mhsmith commented 1 year ago

Future development of the Android event loop will take place in Toga itself, so I'll move this issue to the Toga project.