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

Command responses are random #81

Closed flukas closed 7 years ago

flukas commented 8 years ago

Commands cause random seemingly unrelated errors (eg. Expected key 'volume', got 'repeat' for Adding to playlist)

rnixx commented 8 years ago

Can you provide an example to reproduce please?

flukas commented 8 years ago

It only happens sometimes. Could it be caused by multiple threads accessing the same MPDClient?

10:13:31.936054 - audio-MainThread (info): Adding error from callback <function OnMessage at 0x764ea390>: [50@0] {add} Malformed URI error from callback <function OnMessage at 0x764ea390>: [2@0] {play} Bad song index error from callback <function OnMessage at 0x764ea390>: [2@0] {play} Bad song index error from callback <function OnMessage at 0x764ea390>: [2@0] {play} Bad song index error from callback <function OnMessage at 0x764ea390>: Expected key 'volume', got 'repeat' 10:38:01.929073 - audio-MainThread (info): Updating DB 10:38:01.951133 - audio-MainThread (info): None 10:38:01.961026 - audio-MainThread (info): Listing 10:38:02.538109 - audio-MainThread (info): [{'volume': '77', 'repeat': '0', 'consume': '0', 'random': '0', 'single': '0'}, {'mixrampdb': '0.000000', 'playlistlength': '0', 'state': 'stop', 'playlist': '2'}] 10:38:02.544792 - audio-MainThread (info): Adding error from callback <function OnMessage at 0x764ea390>: Got unexpected return value: 'volume: 77' Exception in thread audioUpdater: Traceback (most recent call last): File "/usr/lib/python3.4/threading.py", line 920, in _bootstrap_inner self.run() File "/usr/lib/python3.4/threading.py", line 868, in run self._target(_self._args, _self._kwargs) File "./server/audio.py", line 315, in run status = mpdc.status() File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 629, in decorator return wrapper(self, name, args, bound_decorator(self, returnValue)) File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 254, in _execute return retval() File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 623, in decorator return function(self, _args, _kwargs) File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 404, in _fetch_object objs = list(self._read_objects()) File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 332, in _read_objects for key, value in self._read_pairs(): File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 311, in _read_pairs pair = self._read_pair(separator) File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 302, in _read_pair line = self._read_line() File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 291, in _read_line raise CommandError(error) mpd.CommandError: [50@0] {add} Malformed URI audio.py.txt

pcwalden commented 7 years ago

Could it be caused by multiple threads accessing the same MPDClient?

Yes it could. Nothing is thread-safe. The application must use mutexes to prevent collisions.

flukas commented 7 years ago

Thanks for reply. Would it help then to create more independent clients - one for each thread?

pcwalden commented 7 years ago

Would it help then to create more independent clients - one for each thread?

That I would not know. My guess is that there are probably global data variables that would be trashed even if multiple clients are spawned for each thread.

Someone with a deeper understanding of the internals would need to respond. I just create a wrapper to protect the client.

class LockableMPDClient(MPDClient):
    def __init__(self, use_unicode=False):
        super(LockableMPDClient, self).__init__()
        self.use_unicode = use_unicode
        self._lock = Lock()
    def acquire(self):
        self._lock.acquire()
    def release(self):
        self._lock.release()
    def __enter__(self):
        self.acquire()
    def __exit__(self, type, value, traceback):
        self.release()

Usage is:

MPD = LockableMPDClient() # for sending commands and getting status
with MPD:
     ..stuff using the locked MPD...

I use it in my RaspberryPi mpdremote client.

Mic92 commented 7 years ago

@flukas yes, one client per thread is safe. There are no shared variables between instances. The locking example can be also found in example section: https://github.com/Mic92/python-mpd2/blob/master/examples/locking.py

Mic92 commented 7 years ago

Beside locking, which can implemented on top of python-mpd, I don't see any actionable insights for this issue. The absence of thread safety is documented here: https://pythonhosted.org/python-mpd2/topics/advanced.html?highlight=thread#thread-safety Therefor I close this issue now.