Mic92 / python-mpd2

Python library which provides a client interface for the Music Player Daemon.
GNU Lesser General Public License v3.0
352 stars 119 forks source link

Pending commands hang forever if connection breaks while reading MPD's response #220

Closed 2franix closed 11 months ago

2franix commented 12 months ago

The present bug happens while asyncio.MPDClient.__run() is handling one of the command futures waiting in the MPDClient.__command_queue. If the MPD server happens to become unreachable while calling result._feed_from(self) (as of writing, this is here), a ConnectionError (or a subclass of) is raised which is caught by this last resort exception handler.

The handler takes care of disconnecting the client and propagates the exception to the future being handled. Unfortunately, it does not propagate it to the other futures still waiting in the command queue. Therefore, those commands will never reach the done state and any client awaiting the command's result will block forever.

I reproduced the bug with versions 3.0.5 and 3.1.0. I did not try any other versions, so I can't tell. Reproduction steps must be followed with care, as the connection error has to happen when reading the server response, not when sending the command.

Here is a minimal script I put together to reproduce the issue. It does a first successful call to status() before replacing the method reading from MPD with a modified one simulating a connection error. The next call to status() will then fail and the client will never see that exception.

from mpd.asyncio import MPDClient, BaseCommandResult
import asyncio

async def rigged_feed_from(command, mpd_client):
    line = await mpd_client._read_line()
    raise ConnectionError()

async def fetch_status(client):
    print("Fetching status...")
    status = await client.status()
    print(f"Status: {status}")

async def main_async():
    client = MPDClient()
    client.timeout = 30
    client.idletimeout = None
    await client.connect("localhost", 6600)

    # Do a first successful iteration.
    await fetch_status(client)

    # Simulate a connection error when reading from MPD.
    BaseCommandResult._feed_from = rigged_feed_from

    # Allow some time for a new idle command to start with the rigged class.
    # Next time it reads from MPD, a connection error will be raised,
    await asyncio.sleep(0.6)

    try:
        # Fetch again.
        await fetch_status(client)
    except ConnectionError:
        # Never reached! Instead, the exception kills the asyncio loop.
        print("Connection failure!")

    # This line is never reached.
    print("Done!")

asyncio.run(main_async())

When run, the output is:

Fetching status...
Status: {'repeat': '0', 'random': '0', 'single': '0', 'consume': '0', 'partition': 'default', 'playlist': '8', 'playlistlength': '10', 'mixrampdb': '0.000000', 'state': 'stop'}
Fetching status...
Exception in callback MPDClient.__idle_result(<CommandResul...ectionError()>)
handle: <Handle MPDClient.__idle_result(<CommandResul...ectionError()>)>
Traceback (most recent call last):
  File "/home/cyrille/.pyenv/versions/3.11.4/lib/python3.11/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/home/cyrille/develop/python-mpd2/mpd/asyncio.py", line 335, in __idle_result
    idle_changes = result.result()
                   ^^^^^^^^^^^^^^^
  File "/home/cyrille/develop/python-mpd2/mpd/asyncio.py", line 309, in __run
    await result._feed_from(self)
  File "/home/cyrille/tmp/testmpd.py", line 6, in rigged_feed_from
    raise ConnectionError()
ConnectionError

The program ends abruptly because the exception escapes MPDClient.__run(), hence killing the task. Instead, we would like the exception to be caught by the exception handler in the script.

In this scenario, the connection error is raised while reading MPD's response for the idle command here. The command queue contains the "status" command which never gets the exception.

I already have a fix for this problem, which I will submit in a PR shortly.