Pycord-Development / pycord

Pycord, a maintained fork of discord.py, is a python wrapper for the Discord API
https://docs.pycord.dev
MIT License
2.69k stars 459 forks source link

Task garbage collection - Heisenbug #2179

Open davidhozic opened 1 year ago

davidhozic commented 1 year ago

Summary

When creating tasks with create_task, PyCord does not save a reference to a task.

Reproduction Steps

When creating tasks with create_task, PyCord does not save a reference to a task, making it a target for the garbage collector, if the task is doing long and complex work.

This is well documented by the asyncio.create_task:

Save a reference to the result of this function, to avoid a task disappearing mid-execution. The event loop only keeps weak references to tasks. A task that isn’t referenced elsewhere may get garbage collected at any time, even before it’s done. For reliable “fire-and-forget” background tasks, gather them in a collection

This can eventually result in a "Task was destroyed but it is pending!" error being printed out on screen, because the task was garbage collected. It could also just go though without warning.

Minimal Reproducible Code

Hard to reproduce, it's a Heisenbug, but definitely possible.

Expected Results

The library should save references to task and only remove the reference after the task has been finished.

Actual Results

It doesn't, giving opportunity for the task to be destroyed during execution.

Intents

No intents were passed

System Information

Checklist

Additional Context

No response

Haptein commented 5 months ago

I am experiencing precisely this bug and I got to the same conclusion after doing some research! I'm getting one of these every one or two days at random, usually on slash commands that take a while to run (after responding the interaction).

Task was destroyed but it is pending!
task: <Task pending name='pycord: on_interaction' coro=<Client._run_event() done, defined at /home/baefloral/.miniconda3/envs/onboardbot/lib/python
3.8/site-packages/discord/client.py:370> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7faece077fd0>()]>>

In some cases, the tasks appear to get garbage collected while awaiting one or other time consuming coroutine. This usually results in getting GeneratorExit raised as well.

Haptein commented 5 months ago

With some guidance I could attempt a fix as I'm not very familiar with pycord's codebase.

Please correct me if I'm wrong, but in the case of event scheduling wouldn't storing the task in a set (possibly named Client.event_scheduled_tasks), and adding a done callback that removes the task from the set before returning it here be enough? https://github.com/Pycord-Development/pycord/blob/845f7c7d7250887d62bbf65d76468e5d45263773/discord/client.py#L418

champymarty commented 5 months ago

I have saw that same error happening for the on_ready event. But not very consistent or easy to reproduce. It did it a lot more in on_connect when I was trying to initialte the db connections(had to do the db init somwhere else)

Vondyy commented 5 months ago

I have the same issue

llu13701 commented 5 months ago

I am also facing the same issues.. pretty bad that some users thought our app doesnt work after failed 3 times in a row

Haptein commented 4 months ago

Okay it seems like this problem is coming up every time with a specific command in my bot now (it is a bit long running), I'm not sure what changed (code was kept the same for this command).

With some guidance I could attempt a pr for this. @Lulalaby @Dorukyum

With some guidance I could attempt a fix as I'm not very familiar with pycord's codebase.

Please correct me if I'm wrong, but in the case of event scheduling wouldn't storing the task in a set (possibly named Client.event_scheduled_tasks), and adding a done callback that removes the task from the set before returning it here be enough?

https://github.com/Pycord-Development/pycord/blob/845f7c7d7250887d62bbf65d76468e5d45263773/discord/client.py#L418

Lulalaby commented 4 months ago

I'm unsure about that tbh. @Pycord-Development/contributors

davidhozic commented 4 months ago

With some guidance I could attempt a fix as I'm not very familiar with pycord's codebase.

Please correct me if I'm wrong, but in the case of event scheduling wouldn't storing the task in a set (possibly named Client.event_scheduled_tasks), and adding a done callback that removes the task from the set before returning it here be enough?

https://github.com/Pycord-Development/pycord/blob/845f7c7d7250887d62bbf65d76468e5d45263773/discord/client.py#L418

That would work. Just make sure the tasks get removed from the set (through a done callback) after they're done. There are also 2 different client classes that need to be handled with this I think, so not just discord.Client?

Dorukyum commented 3 weeks ago

Tasks should be stored and removed afterwards as discussed. @Haptein would you like to work on this?