asyncioreactor: make sure task isn't deleted midway

[fruch]

in push function, self._loop.create_task is called and it's return value is ignored. While the tests may pass now, this code is not correct and this example is called out in docs as a source of bugs, as python docs suggests.

Ref: https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task

[Lorak-mmk]

This needs to be a member, accessed by self.background_tasks, no? Otherwise it can be garbage collected as well (unless PYthon's garbage collector works in some non-obvious way)

[fruch]

python GC works with ref counting, and since this is referenced in the now in the task callback it won't be evicted

saving it on the instance might be problematic, since this can be called multiple times, and cause deletion of all of the tasks added

[Lorak-mmk]

so Python GC can't deal with cyclic references?

[fruch]

it can handle cyclic references, but I was referring to what you suggested, of saving the list of task on the instance:

    def __init__(self):
            background_tasks = set()

    def push(self):
            ....

            # avoid races/hangs by just scheduling this, not using threadsafe
            task = self._loop.create_task(self._push_msg(chunks))
            
            self.background_tasks.add(task)
            task.add_done_callback(self.background_tasks.discard)

this can end up exact the same issue this PR is trying to elevate, of task object getting cleared midway, if we might have multiple calls to push, there would be multiple tasks in the set, and all of them would be discarded at the same time, when the first task would end.

[Lorak-mmk]

Hmm... I don't follow why it would delete all items - set.discard removed specified item, and callback receives finished task as argument, so will remove only this task from set.

Even in python docs example background_tasks is a global variable

[fruch]

o.k., I've was confused, and thought .clear was used

changed it