[x] I have tested my changes against the dev branch (the latest developmental version), and this pull request is targeting that branch as a base
[x] I have tested my changes on Python 3.10 or higher
[x] I have ensured my code is formatted using Black
Description
This PR makes further changes to playback processing to avoid the infinite loop / deadlock issue created by too many concurrent playback calls waiting for the async player lock.
Some changes have been made to how player errors are handled, to better ensure proper handling of those errors and continue playback where needed. This includes a minor change to YtdlResponseDict objects to provide an input_subject property which can reliably be used for managing autoplaylist entry removal in the player error handling function.
All tasks spawned through asyncio.create_task() in MusicBot now use a method provided by MusicBot.create_task() so MusicBot can manage the reference to said task and ensure it does not get garbage collected before it is finished. This should increase overall stability, though early deletion of tasks may be rare and/or depend on python version. A few exceptions are made for functions outside of MusicBot, such as those related to signal handling.
Processing of shutdown has also been changed. The downloader thread pool executor is now sent a shutdown() which will cancel all pending tasks that have not yet started, but wait for all running threads to finish before moving on with the rest of shutdown. This allows those threads and their system resources to be properly cleaned up, but may cause delays in shutdown or restart when large media files have begun downloading. (Due to how we use ytdlp, we cannot actually cancel or cleanly interrupt the downloads that are running.)
Additionally, the stderr reader for FFmpeg will now read stderr stream for the duration of playback. Reader loop is cancelled by the done() state of the associated asyncio.Future() which is ultimately closed by the _playback_finished callback if no exception was set by the stream reader. This should enable catching and logging errors and warnings from ffmpeg (assuming they are placed in stderr io stream) for the duration of playback, not just the very start.
Finally, minor changes to logging (mostly debug output) have been made to provide more information about the various entry types and their primary data points. Some handling was added to safe_*_message functions to handle aiohttp client errors more gracefully.
Lets hope this PR contains minimal regressions... Only more testing will tell!
devbranch (the latest developmental version), and this pull request is targeting that branch as a baseDescription
This PR makes further changes to playback processing to avoid the infinite loop / deadlock issue created by too many concurrent playback calls waiting for the async player lock.
Some changes have been made to how player errors are handled, to better ensure proper handling of those errors and continue playback where needed. This includes a minor change to YtdlResponseDict objects to provide an
input_subjectproperty which can reliably be used for managing autoplaylist entry removal in the player error handling function.All tasks spawned through
asyncio.create_task()in MusicBot now use a method provided byMusicBot.create_task()so MusicBot can manage the reference to said task and ensure it does not get garbage collected before it is finished. This should increase overall stability, though early deletion of tasks may be rare and/or depend on python version. A few exceptions are made for functions outside of MusicBot, such as those related to signal handling.Processing of shutdown has also been changed. The downloader thread pool executor is now sent a
shutdown()which will cancel all pending tasks that have not yet started, but wait for all running threads to finish before moving on with the rest of shutdown. This allows those threads and their system resources to be properly cleaned up, but may cause delays in shutdown or restart when large media files have begun downloading. (Due to how we use ytdlp, we cannot actually cancel or cleanly interrupt the downloads that are running.)Additionally, the stderr reader for FFmpeg will now read stderr stream for the duration of playback. Reader loop is cancelled by the
done()state of the associatedasyncio.Future()which is ultimately closed by the _playback_finished callback if no exception was set by the stream reader. This should enable catching and logging errors and warnings from ffmpeg (assuming they are placed in stderr io stream) for the duration of playback, not just the very start.Finally, minor changes to logging (mostly debug output) have been made to provide more information about the various entry types and their primary data points. Some handling was added to
safe_*_messagefunctions to handle aiohttp client errors more gracefully.Lets hope this PR contains minimal regressions... Only more testing will tell!