libcurl 8.5.0 breaks mpd http streaming

[bubbleguuum]

I did this

libcurl v8.5.0 is causing issues in mpd http streaming, for http streams whose size is > 128 MB (which causes mpd to not fully buffer the stream in memory). Shortly after playing such stream, a curl API inexplicably returns CURLE_OUT_OF_MEMORY, causing mpd to immediately stop playback. libcurl v8.4.0 and prior work just fine.

I reported all the details and findings about it and how to reproduce it on this mpd issue.

The problematic commit has been found to be 47f5b1a via bissecting.

After analysis, the mpd developer thinks this is a curl issue, hence this report.

Interestingly, mpd running with git libcurl (as of the date of this post) do not cause an OOM but instead will make the mpd "io" thread (making curl API calls) to take 100% CPU.

Any insights for fixing this issue would be appreciated.

I expected the following

No response

curl/libcurl version

libcurl 8.5.0

operating system

openSUSE Tumbleed (this issue is not OS dependent).

[bagder]

Pointing to a cryptic issue in another application is not helping us much. Please describe a lot more about the actual problem and how we can reproduce it - without running or debugging someone else's complicated application.

[bubbleguuum]

mpd developer: not my problem
curl developer: not my problem

After spending several hours on this issue to narrow it down and writing reports, I'm sick of it. Feel free to close.
Let's keep mpd crashing.

[bagder]

A problem with curl is a problem for us, but for us to be able to work on a reported issue we need some guidance - a detailed description to start with. If neither you nor any other mpd user/developer want to help with that, then I cannot see how we can act on this report. Then it will be closed, yes. We are not mind-readers, unfortunately.

[bubbleguuum]

If you want more details, ask mpd developer MaxKellerman, assuming he his willing to help, which I doubt.
I've done my part at this point.

[bagder]

Sorry, but if nobody can describe a curl problem to us then there is no curl problem as far as I am concerned.

[bubbleguuum]

Did you read this analysis from the mpd developer with the change in behavior between 6.4.0 and 6.5.0:

MusicPlayerDaemon/MPD#1952 (comment)

[bagder]

That's a description of mpd behavior when it uses libcurl together with some strace dumps. I have exactly zero knowledge of what mpd does with libcurl or how it works in general so I cannot tell what most of that means in terms of curl problems or not. What APIs does it use? What protocol (version) does it speak? What exactly is mpd doing when this issue is triggered? Which libcurl function returns what error?

[bubbleguuum]

All I know is that mpd uses the easy API for http streaming. I'll leave it to developers of 2 large software to eventually figure it out. I'm expert in neither the curl nor mpd codebase.

[jay]

@MaxKellermann

[MaxKellermann]

MPD uses the CURL "multi" API and uses CURLMOPT_SOCKETFUNCTION to integrate it in its epoll-based I/O event loop. See https://github.com/MusicPlayerDaemon/MPD/blob/master/src/lib/curl/Global.cxx for the part that does this integration. There is nothing special here, I guess every program using CURL does it that way, that's how CURL is supposed to be integrated in non-blocking I/O loops, is it not?

So the problem is that returning CURL_WRITEFUNC_PAUSE used to lead to a CURLMOPT_SOCKETFUNCTION call that would unregister the HTTP connection socket, effectively pausing the transfer (in CURL 8.4.0).
That CURLMOPT_SOCKETFUNCTION is no longer performed by CURL 8.5.0, and instead, CURL keeps receiving from the "paused" socket, buffering received data until the buffer eventually grows too large, causing dyn_nappend() to return CURLE_OUT_OF_MEMORY, failing the whole HTTP request.

I expect CURL_WRITEFUNC_PAUSE to effectively pause the transfer and never grow the dynbuf too large.

CURL 8.5.0 does not actually pause the transfer and not receive any data; but all it actually does since 8.5.0 is omit the WRITEFUNCTION calls. But it really keeps receiving+buffering data.

This is a CURL regression; at best, this is a semantic ABI change which breaks at least one widely used application after the update. But I believe this is really a bug, not just some minor semantic change.

[bagder]

@MaxKellermann HTTP/1 or HTTP/2 ?

[MaxKellermann]

I only tested with 1.

[icing]

@MaxKellermann it would be lovely if you could check #12633 out for a test.

[MaxKellermann]

@icing with your patch on top of 8.5.0, the problem is gone and MPD can play the song.

[bubbleguuum]

And I confirm patch works with libcurl master as well (without it, this issue results in 100% CPU usage in the mpd io thread).
Thanks for the fix!