MPD sometimes dead lock?

[Rio6]

I noticed sometimes MPD deadlocks and become unresponsive (mpc commands failed with timeout, ctrl-c on MPD process does not terminate it). I haven't figured out what exactly causes that. Backtrace in gdb shows these two locations:

(gdb) bt
#0  0x00007ffff45dddb0 in __lll_lock_wait () at /usr/lib/libpthread.so.0
#1  0x00007ffff45d6743 in pthread_mutex_lock () at /usr/lib/libpthread.so.0
#2  0x0000555555594182 in __gthread_mutex_lock(__gthread_mutex_t*) (__mutex=0x5555558f6758) at /usr/include/c++/10.2.0/x86_64-pc-linux-gnu/bits/gthr-default.h:749
#3  0x000055555559431e in std::mutex::lock() (this=0x5555558f6758) at /usr/include/c++/10.2.0/bits/std_mutex.h:100
#4  0x000055555559753b in std::unique_lock<std::mutex>::lock() (this=0x7fffffffd250) at /usr/include/c++/10.2.0/bits/unique_lock.h:138
#5  0x00005555555972a5 in std::unique_lock<std::mutex>::unique_lock(std::mutex&) (this=0x7fffffffd250, __m=...) at /usr/include/c++/10.2.0/bits/unique_lock.h:68
#6  0x00005555555bd1a2 in PlayerControl::LockGetStatus() (this=0x5555558f6708) at ../../src/player/Control.cxx:172

(gdb) bt
#0  0x00007ffff45dddb0 in __lll_lock_wait () at /usr/lib/libpthread.so.0
#1  0x00007ffff45d6743 in pthread_mutex_lock () at /usr/lib/libpthread.so.0
#2  0x0000555555594182 in __gthread_mutex_lock(__gthread_mutex_t*) (__mutex=0x5555558f6758) at /usr/include/c++/10.2.0/x86_64-pc-linux-gnu/bits/gthr-default.h:749
#3  0x000055555559431e in std::mutex::lock() (this=0x5555558f6758) at /usr/include/c++/10.2.0/bits/std_mutex.h:100
#4  0x00005555555947c6 in std::lock_guard<std::mutex>::lock_guard(std::mutex&) (this=0x7fffed7ba3a0, __m=...) at /usr/include/c++/10.2.0/bits/std_mutex.h:159
#5  0x0000555555639a07 in CurlInputStream::OnData(ConstBuffer<void>) (this=0x7fffdc0024d0, data=...) at ../../src/input/plugins/CurlInputPlugin.cxx:305

Update: they're probably waiting for the other two threads:

(gdb) bt
#0  0x00007ffff45da6a2 in pthread_cond_wait@@GLIBC_2.3.2 () at /usr/lib/libpthread.so.0
#1  0x00007ffff4816c11 in __gthread_cond_wait (__mutex=<optimized out>, __cond=<optimized out>)
    at /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:865
#2  std::condition_variable::wait(std::unique_lock<std::mutex>&) (this=<optimized out>, __lock=<optimized out>) at /build/gcc/src/gcc/libstdc++-v3/src/c++11/condition_variable.cc:53
#3  0x00005555555a1c53 in DecoderControl::WaitForDecoder(std::unique_lock<std::mutex>&) (this=0x7fffe7ffe770, lock=...) at ../../src/decoder/Control.hxx:224
#4  0x00005555555b9661 in Player::CheckDecoderStartup(std::unique_lock<std::mutex>&) (this=0x7fffe7ffe670, lock=...) at ../../src/player/Thread.cxx:572

(gdb) bt
#0  0x00007ffff45da6a2 in pthread_cond_wait@@GLIBC_2.3.2 () at /usr/lib/libpthread.so.0
#1  0x00007ffff4816c11 in __gthread_cond_wait (__mutex=<optimized out>, __cond=<optimized out>)
    at /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:865
#2  std::condition_variable::wait(std::unique_lock<std::mutex>&) (this=<optimized out>, __lock=<optimized out>) at /build/gcc/src/gcc/libstdc++-v3/src/c++11/condition_variable.cc:53
#3  0x000055555574944c in std::condition_variable::wait<BlockingCallMonitor::Run()::{lambda()#1}>(std::unique_lock<std::mutex>&, BlockingCallMonitor::Run()::{lambda()#1})
    (this=0x7fffe77fcf10, __lock=..., __p=...) at /usr/include/c++/10.2.0/condition_variable:111
#4  0x00005555557491bb in BlockingCallMonitor::Run() (this=0x7fffe77fcea0) at ../../src/event/Call.cxx:54
#5  0x0000555555749036 in BlockingCall(EventLoop&, std::function<void ()>&&) (loop=..., f=...) at ../../src/event/Call.cxx:88
#6  0x0000555555669328 in Ytdl::InvokeContext::~InvokeContext() (this=0x7fffd80012d0, __in_chrg=<optimized out>) at ../../src/lib/ytdl/Invoke.cxx:218

Not sure how they're realated to each other yet.

[Rio6]

This is on ytdl branch btw

[Rio6]

It looks like it happens only on videos where youtube-dl returns a "https://manifest.googlevideo.com/api/manifest/dash/..." as the video URL. Videos with "https://r7---sn-gvbxgn-tt1e.googlevideo.com/videoplayback" URL works. Passing --youtube-skip-dash-manifest to youtube-dl it seems to fix the problem. I think we should add that as part of the invoke command.

Update: using bestaudio[protocol=https]/best as format also works.

More update: directly playing youtube dash url with mpc add plays it normally (I think it's using ffmpeg plugin), so I guess using ffmpeg instead of curl as backend is another option.

Although the code should still be fixed so it doesn't completely lock up MPD when something similiar happens.

[Rio6]

I think it's caused by here:

size_t YtdlInputStream::Read(std::unique_lock<Mutex> &lock, void *ptr, size_t sz) {
    if (input != nullptr) {
        context = nullptr;
    }
    return ProxyInputStream::Read(lock, ptr, sz);
}

The line context = nullptr; calls the destructor which runs things in the event loop and blocks. And something about that locks everything up. Removing that line actually allows the dash URLs to play (although they lag more compared to http streams, not sure if it's just me).

Maybe it needs to be moved elsewhere.

[arcnmx]

Hm, it might be fine to move it back into OnComplete, just try reverting b784c3b229451e5df79ff372fde9ec380aaa5154. I moved it because it seemed dangerous but it's technically valid as long as we're careful?

[Rio6]

Making it to be the last thing to do in OnComplete (and OnError) looks fine for now. OnComplete is then the last thing being called in YtdlMonitor::OnSocketReady, which is the last thing in SocketEvent::Dispatch, which get's called at the end of its scope in EventLoop::Run.

Not sure if that's going to be guaranteed in the future though.

[arcnmx]

Not sure if that's going to be guaranteed in the future though.

It's probably going to be for SocketEvent::Dispatch, since it's not unusual for the event to unregister itself as part of that dispatch handler. The rest is controlled by this plugin and is pretty reasonable to keep it that way so should be fine.