samba subtitles and crashing (thread-unsafe libsmbclient used across threads -> crash) #5936
Comments
|
This particular libsmbclient code in mpv causes crashes: with many various libsmbclient messages, some of which are: This is with either the latest upstream (git mirror) or 4.7.7 of samba, though all logs and stack traces here are from the latest version. Here is a trace from gdb, though I'm not sure if these vary as much as the above messages: I've tried duplicating the crash with this minimal example: Which does duplicate the original stack trace, but without a crash: Which leads to several shot-in-the-dark theories:
|
|
Update (from irc):
If you look at the stack frames before |
|
Running valgrind's memcheck with Running valgrind's helgrind shows many relevant messages like: Or: Which is a little concerning considering that talloc contexts are not thread-safe (without cross-thread synchronization). Also I could never trigger a crash (with either tool) when running under valgrind, though the external remote subtitles always failed to load. |
|
Update:
Just streaming any libsmbclient descriptors creates/frees talloc data. That's why loading the srt as the main file and the mkv with ...And yes that's it: with |
orbisvicis
changed the title
orbisvicis
pushed a commit
to orbisvicis/mpv
that referenced
this issue
Jun 23, 2018
Thread-unsafe libsmbclient crashes when used across threads, for example when provided more than one "smb://" URI. Fix this by locking all calls to this library under a single mutex, which as expected will throttle performance across concurrent smb streams. Subtitles are preloaded and so don't count. This is a temporary solution. When the following libsmbclient bug is fixed, switch to the code from branch `smbclient_threaded_api`: * https://bugzilla.samba.org/show_bug.cgi?id=11413
orbisvicis
pushed a commit
to orbisvicis/mpv
that referenced
this issue
Jun 23, 2018
Thread-unsafe libsmbclient crashes when used across threads, for example when provided more than one "smb://" URI. This patch allows each library function to run concurrently with itself while locking out other library functions, in an attempt to boost the performance of the `smbclient_lock_all` branch. Unfortunately it fails horribly, so just here for posterity.
orbisvicis
pushed a commit
to orbisvicis/mpv
that referenced
this issue
Jun 23, 2018
Thread-unsafe libsmbclient crashes when used across threads, for example when provided more than one "smb://" URI. Switch from the compatibility api to the new libsmbclient api. Each use - presumably per-thread - of `open_f` creates a new smb context, the deallocation of which is tied to the talloc destructor of the stream. As opposed to locking cross-thread calls, the new thread-safe api provides better streaming performance. When the following thread-safety issue is fixed, this should be the preferred solution: * https://bugzilla.samba.org/show_bug.cgi?id=11413
orbisvicis
pushed a commit
to orbisvicis/mpv
that referenced
this issue
Jun 25, 2018
Thread-unsafe libsmbclient is likely to crash when used simultaneously
across threads. For example, by:
mpv "smb://...mkv" --sub-file "smb://...srt"
Work around this by locking all calls to this library under a single
global mutex. This is a temporary solution. When the libsmbclient bug
[1] is fixed, switch to the code from branch [2] which uses a new api to
create per-thread smb contexts. Fixes [3].
[1] https://bugzilla.samba.org/show_bug.cgi?id=11413
[2] https://github.com/orbisvicis/mpv/tree/smbclient_threaded_api
[3] mpv-player#5936
jeeb
pushed a commit
that referenced
this issue
Jul 29, 2018
Thread-unsafe libsmbclient is likely to crash when used simultaneously
across threads. For example, by:
mpv "smb://...mkv" --sub-file "smb://...srt"
Work around this by locking all calls to this library under a single
global mutex. This is a temporary solution. When the libsmbclient bug
[1] is fixed, switch to the code from branch [2] which uses a new api to
create per-thread smb contexts. Fixes [3].
[1] https://bugzilla.samba.org/show_bug.cgi?id=11413
[2] https://github.com/orbisvicis/mpv/tree/smbclient_threaded_api
[3] #5936
jeeb
pushed a commit
that referenced
this issue
Jul 29, 2018
Thread-unsafe libsmbclient is likely to crash when used simultaneously
across threads. For example, by:
mpv "smb://...mkv" --sub-file "smb://...srt"
Work around this by locking all calls to this library under a single
global mutex. This is a temporary solution. When the libsmbclient bug
[1] is fixed, switch to the code from branch [2] which uses a new api to
create per-thread smb contexts. Fixes [3].
[1] https://bugzilla.samba.org/show_bug.cgi?id=11413
[2] https://github.com/orbisvicis/mpv/tree/smbclient_threaded_api
[3] #5936
(cherry picked from commit 112b3fa)
|
Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
mpv version
platform
Reproduction steps
Expected behavior
Load subtitles.
Actual behavior
Even worse...
After I enable smb debugging, mpv now usually crashes when attempting to load the subtitles, even if run without any debugging. For example:
Notice how the video file stream is successfully opened (anonymously), but not the subtitles. The same crash also happens with the repository-provided mpv:
The text was updated successfully, but these errors were encountered: