Concern around deprecating App.add_background_task()
#2809
Comments
|
Well... TIL :-) The primary motivation behind deprecating I agree that it's very curious that this has never been reported as an problem in live Toga apps. My immediate guess is that there's something in the underlying It also raises the question over whether the |
freakboy3742
added
the
awaiting details
|
I read through that long SO answer I referenced. It describes a somewhat convoluted way that the garbage collector can destroy a Task while it is running; and thus demonstrating the need to keep a reference to the Task. So, the situation to create this issue exists...but most implementations probably accidentally avoid it because of exact implementation of A Toga app that creates the issue: import asyncio
import gc
import toga
class HelloWorld(toga.App):
def startup(self):
self.main_window = toga.MainWindow(content=toga.Button("Click"))
self.main_window.show()
self.tasks = [
asyncio.create_task(self.coro1(), name="coro1"),
asyncio.create_task(self.coro2(), name="coro2"),
]
async def on_running(self):
print("inside on_running - going to wait for future")
await self.loop.create_future()
async def coro1(self):
for _ in range(5):
print("coro1 printing...")
await asyncio.sleep(1)
gc.collect()
self.exit()
async def coro2(self):
print("inside coro2 - going to wait for future")
await self.loop.create_future()
def main():
return HelloWorld()When you run it, you can see that the Task created for Notably, the same thing doesn't happen to Given this, I think we should update how Toga's handlers are managed to guarantee that Tasks created by |
|
Tracking issue for CPython There is support for adding native hard references for arbitrary tasks created by |
Agreed - you've provided a relatively simple reproduction case; the implication is that the only reason we haven't seen this problem is that they haven't exercised sufficient garbage collection to expose the problem.
Agreed. There's a documentation TODO covering writing a "how to do async"; #2099 is still open as a general documentation task covering asyncio usage; the topic is partially covered by the Beeware tutorial, but there's room for a lot more discussion. A discussion of retaining tasks would seem to fit well into that documentation topic. |
So, I think we should update the documentation....but what if we just hook |
|
You mean monkeypatch the official method to also do some form of caching of created tasks? Or is there a hook we can use that I'm not thinking of? |
|
Not quite; |
|
Oh - interesting... I hadn't considered that approach. I guess that could work; it might get a little complicated with GTK, especially if we switch to the native GTK event loop... but that might be a viable way to make the call safe. As an aside, I'll also be at the core team summit in a couple of weeks, so I can see about getting some movement on the upstream issue there (as has been suggested on the ticket). That won't fix the situation on Python 3.9-3.12, but it would mean any shim we add could be temporary. |
|
While I was thinking about it...perhaps Toga could set a task factory for the loop; this might be a better supported option to inject some logic and then defer to the loop's default behavior. |
|
I didn't know that option existed - but from the look of it, it would be an even better option. It would guarantee that all tasks created would have lifecycle protection, without the need for any additional handling. |
|
in toga 0.4.6, how to use asyncio.create_task() to replace App.add_background_task(), would u give a example？ @rmartin16 @freakboy3742 |
rmartin16
removed
the
awaiting details
|
Sure; here's an example of an app creating background tasks: class HelloWorld(toga.App):
def startup(self):
main_box = toga.Box()
self.main_window = toga.MainWindow(title=self.formal_name)
self.main_window.content = main_box
self.main_window.show()
self.loop.call_soon_threadsafe(self.sync_task, "Hi")
asyncio.create_task(self.async_task("Hi"))
def sync_task(self, arg):
print(f"running sync task: {arg}")
async def async_task(self, arg):
print(f"running async task: {arg}") |
|
@rmartin16 thanks for your reply, but when run your example code, it will raise "RuntimeError: no running event loop". |
|
There are different ways to start a Toga app; if you aren't using a project created by Briefcase, the code below can start this app (after you've imported asyncio and toga): HelloWorld(formal_name="Hello World", app_id="com.example.helloworld").main_loop() |
|
Also, if you're curious about asyncio outside of Toga, here's similar code: async def main():
asyncio.get_running_loop().call_soon_threadsafe(sync_task, "Hi")
await asyncio.create_task(async_task("Hi"))
def sync_task(arg):
print(f"running sync task: {arg}")
async def async_task(arg):
print(f"running async task: {arg}")
asyncio.run(main()) |
@rmartin16 yes, I run this example code with briefcase and it will raise "RuntimeError: no running event loop". Detail info as below [testtoga] Starting in dev mode...Traceback (most recent call last): Problem running app testtoga. |
|
Ahh, ok, I see now; thank you. On Windows, the event loop is indeed started after Given that, a workaround for the time being is to add async tasks from a sync task if such tasks need to be added from class HelloWorld(toga.App):
def startup(self):
main_box = toga.Box()
self.main_window = toga.MainWindow(title=self.formal_name)
self.main_window.content = main_box
self.main_window.show()
self.loop.call_soon_threadsafe(self.sync_task, "Hi")
def sync_task(self, arg):
print(f"running sync task: {arg}")
asyncio.create_task(self.async_task("Hi"))
async def async_task(self, arg):
print(f"running async task: {arg}") |
|
Also, @Jzhenli, with the latest version of Toga, you can also use an For instance: class HelloWorld(toga.App):
def startup(self):
main_box = toga.Box()
self.main_window = toga.MainWindow(title=self.formal_name)
self.main_window.content = main_box
self.main_window.show()
self.loop.call_soon_threadsafe(self.sync_task, "Hi")
def sync_task(self, arg):
print(f"running sync task: {arg}")
asyncio.create_task(self.async_task("from sync task"))
async def async_task(self, arg):
print(f"running async task: {arg}")
async def on_running(self):
print(f"on_running") |
|
now it works, thanks @rmartin16 |
Describe the bug
In #2678,
add_background_task()was deprecated; users are now recommended to useasyncio.create_task()instead. However,asyncio.create_task()is not technically a sufficient drop-in replacement foradd_background_task().Most existing uses of
add_background_task()are almost certainly just call it and forget it, e.g.:Replacing this blindly with
asyncio.create_task()is not inherently safe. As outlined in the Python documentation and discussed on Discourse, users should take care to ensure a reference to the returnedTaskis maintained until the task completes.So, really, users should rewrite their previous
add_background_task()calls like this:I can imagine this isn't obvious to many developers using Toga.
Steps to reproduce
Use
asyncio.create_task()in an app.Expected behavior
Toga should recommend a method to run background tasks with less potentially sharp edges. Although, I think such a method would look a lot like the previous
add_background_task()...That said, I think that
add_background_task()may have been exposed to this same underlying issue....since it ultimately just callsensure_future()for a wrapped version of the user's coroutine and a reference to theFuturereturned byensure_future()is never maintained.Screenshots
No response
Environment
Logs
No response
Additional context
I think, mostly, I just wanted to capture these thoughts and see what others think. First, the origin of the warning in the Python docs isn't clear to me...namely, when is this important? (maybe this incredibly verbose SO answer says why...) Second, if Toga has always been exposed to this...why hasn't the issue ever been reported?
The text was updated successfully, but these errors were encountered: