Crash while searching

[dgcampea]

If mpdevil gets disconnected from mpd while searching, the search bar is unusable/greyed-out after reconnecting, requiring a program restart in order to unblock it again.

Steps to reproduce:
Search for something and have mpd disconnect while in progress.
Reconnect.
Search bar is greyed out with previous search text.

Extra info:
Before mpdevil gets disconnected, mpd logs show:

"exception: error on client 22: Output buffer is full"

I've asked over #mpd IRC channel and someone suggested that it's most likely a MPD client bug.

[SoongNoonien]

I'm sorry I cannot reproduce this issue. When exactly is "while in progress"? Is the progressbar in the search entry already visible?

[dgcampea]

Sent you an email with screen capture showing the bug.

[SoongNoonien]

Thanks for the detailed info. This should now be fixed in master. I'm sadly not able to test it since my collection seemingly is way to small. I just can't hint the right moment to crash the player, the search is always to fast. I patched it to always "crash" while searching and this didn't crash the player anymore. So please let me know if it fixed on your machine.

[dgcampea]

I can confirm 14d4766 fixes the greyed out search box but I'm still being instantly disconnected as soon as I try to search something.
Doesn't seem to be a mpd issue as I'm able to search using mpc without slowdown or crashing (in fact, it's almost instantaneous).

[SoongNoonien]

Ok, sorry I misunderstood you. I thought you stopped MPD on purpose.

Doesn't seem to be a mpd issue as I'm able to search using mpc without slowdown or crashing (in fact, it's almost instantaneous).

On my machine the actual mpd search is also almost instantaneous. Is it possible for you to test the search directly with python-mpd2?

[dgcampea]

On my machine the actual mpd search is also almost instantaneous. Is it possible for you to test the search directly with python-mpd2?

>>> from mpd import MPDClient
>>> client = MPDClient()
>>> client.connect("localhost", 6600)
>>> client.find("title", "spend winter")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 469, in mpd_command
    return wrapper(self, name, args, callback)
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 532, in _execute
    return retval()
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 457, in command_callback
    res = self._wrap_iterator(res)
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 706, in _wrap_iterator
    return list(iterator)
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 240, in _parse_objects
    for key, value in self._parse_pairs(lines):
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 235, in _parse_pairs
    for line in lines:
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 586, in _read_lines
    line = self._read_line()
  File "/app/lib/python3.8/site-packages/mpd/base.py", line 571, in _read_line
    raise ConnectionError("Connection lost while reading line")
mpd.base.ConnectionError: Connection lost while reading line

[SoongNoonien]

Ok, so I'd consider this a bug in python-mpd2. What do you think?

[dgcampea]

Ok, so I'd consider this a bug in python-mpd2. What do you think?

Yeah, I agree. Do you want me to open a ticket there?

[SoongNoonien]

Yes, that's a good idea. I can't report a bug I'm not experiencing. I'll close this for now, feel free to reopen when additional work on this is needed.

[dgcampea]

Opened Mic92/python-mpd2#174.

[dgcampea]

@SoongNoonien By chance, what mpd version are you using?

[SoongNoonien]

I'm using MPD 0.22.9 and python-mpd2 3.0.4.

[dgcampea]

I'm using MPD 0.22.9 and python-mpd2 3.0.4.

And search is working for you without disconnects?

[SoongNoonien]

And search is working for you without disconnects?

Well, yes obviously if not I would have left this bug open. ;-) And why should I implement a search which is not even working on my own machine? :-)

[dgcampea]

OK.
I'm investigating this right now and so far it looks like it's not a single bug here, we're looking at multiple ones across different components.

[dgcampea]

I think this can be reopened, there's things that mpdevil can do here that won't cause disconnections.

[SoongNoonien]

I think this can be reopened, there's things that mpdevil can do here that won't cause disconnections.

And which are those?

[dgcampea]

OK, I've got the search to actually work (still requires some refactoring as it's just a bunch of hacks atm).
Here's what I found:

• The bug is happening because the results are so numerous we are hitting mpd buffer limit here which causes it to disconnect.
• The reason mpc doesn't crash is that mpc is smart and actually is requesting a much smaller subset of data (this is controlled by tagtypes. mpc just requests no tags at all)
• But we can't request no tags at all, some are required for displaying in the UI (such as Artist, Title, etc.)
• Requesting a smaller subset does help mitigate the problem here but the data over here was so large it still exceeded the buffer limit.
• We can use "window" property that mpd protocol search and find have to control how many results per request we get. This allows to bunch them up via a loop
  • Naively doing a while loop to collect all the data in portions will actually fully block mpdevil GUI. This will require some extra effort to avoid.

An interesting find was that if you type fast enough or paste the text to search you won't crash the search presumably because the more you type the less results there will be.

[dgcampea]

As a sidenote, python-mpd2 shouldn't be disconnecting in the code example that was initially posted, it's crashing on a empty search result and the command there is a valid mpd protocol one (try with netcat and you'll see mpd respond with OK)

[SoongNoonien]

This should be doable. I'm currently displaying the search results in packages of 100 songs it should be no problem to also fetch them in packages/windows of 100. After each package the GUI gets refreshed, so it does not block.

[SoongNoonien]

c520e43 should fix this even with the crashing python-mpd2. It is now doing exactly what I described in my previous post. Can you verify that? I'll possibly use the tagtypes command as well here in the future, maybe when working on #39.

[dgcampea]

I can confirm this fixes the issue and the UI doesn't block anymore.
You can close this issue though I'm investigating if further (performance) improvements can be done here.