GNOME Bugzilla – Bug 796731
gstplayer join to self cause crash
Last modified: 2018-07-18 15:47:46 UTC
Based on gstplayer of app on Android, when we play/exit repeatedly, there may be crash(an "error 'Resource deadlock would occur' ") --- 07-02 09:51:49.876 21512 21512 F DEBUG : ABI: 'arm' 07-02 09:51:49.876 21512 21512 F DEBUG : pid: 20491, tid: 21488, name: GstPlayer 07-02 09:51:49.876 21512 21512 F DEBUG : signal 5 (SIGTRAP), code -6 (SI_TKILL), fault addr -------- 07-02 09:51:49.876 21512 21512 F DEBUG : r0 00000000 r1 000053f0 r2 00000005 r3 ffffffff 07-02 09:51:49.876 21512 21512 F DEBUG : r4 ffffffff r5 00000006 r6 ec2774cc r7 0000010c 07-02 09:51:49.876 21512 21512 F DEBUG : r8 e7ad0400 r9 ec2774cc sl ed12b068 fp 00000000 07-02 09:51:49.876 21512 21512 F DEBUG : ip 00000004 sp e53ff6a8 lr ebf65523 pc ee5afa94 cpsr 00010010 07-02 09:51:49.884 21512 21512 F DEBUG : 07-02 09:51:49.884 21512 21512 F DEBUG : backtrace: 07-02 09:51:49.884 21512 21512 F DEBUG : #00 pc 0004aa94 /system/lib/libc.so (tgkill+12) 07-02 09:51:49.884 21512 21512 F DEBUG : #01 pc 014b951f /system/lib/libgstreamer_android.so (g_log_default_handler+86) 07-02 09:51:49.884 21512 21512 F DEBUG : #02 pc 014b9705 /system/lib/libgstreamer_android.so (g_logv+456) 07-02 09:51:49.884 21512 21512 F DEBUG : #03 pc 014b97ad /system/lib/libgstreamer_android.so (g_log+12) 07-02 09:51:49.884 21512 21512 F DEBUG : #04 pc 014dc37d /system/lib/libgstreamer_android.so 07-02 09:51:49.884 21512 21512 F DEBUG : #05 pc 014cb0e9 /system/lib/libgstreamer_android.so (g_thread_join+52) 07-02 09:51:49.884 21512 21512 F DEBUG : #06 pc 00014745 /system/lib/libmmpadapter.so 07-02 09:51:49.884 21512 21512 F DEBUG : #07 pc 01484cd1 /system/lib/libgstreamer_android.so (g_object_unref+212) 07-02 09:51:49.884 21512 21512 F DEBUG : #08 pc 000122ad /system/lib/libmmpadapter.so 07-02 09:51:49.884 21512 21512 F DEBUG : #09 pc 00017cc7 /system/lib/libmmpadapter.so 07-02 09:51:49.884 21512 21512 F DEBUG : #10 pc 000183cb /system/lib/libmmpadapter.so 07-02 09:51:49.884 21512 21512 F DEBUG : #11 pc 000185c9 /system/lib/libmmpadapter.so 07-02 09:51:49.884 21512 21512 F DEBUG : #12 pc 014b57ff /system/lib/libgstreamer_android.so (g_main_context_dispatch+206) 07-02 09:51:49.884 21512 21512 F DEBUG : #13 pc 014b59a1 /system/lib/libgstreamer_android.so 07-02 09:51:49.884 21512 21512 F DEBUG : #14 pc 014b5c49 /system/lib/libgstreamer_android.so (g_main_loop_run+204) 07-02 09:51:49.884 21512 21512 F DEBUG : #15 pc 000180ad /system/lib/libmmpadapter.so 07-02 09:51:49.885 21512 21512 F DEBUG : #16 pc 014cad8d /system/lib/libgstreamer_android.so 07-02 09:51:49.885 21512 21512 F DEBUG : #17 pc 00047db7 /system/lib/libc.so (_ZL15__pthread_startPv+22) 07-02 09:51:49.885 21512 21512 F DEBUG : #18 pc 0001af7f /system/lib/libc.so (__start_thread+32) Add a line log tells us that sometimes we might call a join on the same thread, ---- static void gst_player_dispose (GObject * object) { GstPlayer *self = GST_PLAYER (object); GST_TRACE_OBJECT (self, "Stopping main thread"); *if (self->thread == g_thread_self()) * __android_log_print(ANDROID_LOG_DEBUG, "####", "Oooops,you cannot join to yourself!!!!"); ... if (self->loop) { g_main_loop_quit (self->loop); g_thread_join(self->thread); ... } You cannot join to yourself. The POSIX manpage for pthread_join specifies(http://pubs.opengroup.org/onlinepubs/7908799/xsh/pthread_join.html) that you may get a deadlock error, So we call change_state (self, GST_PLAYER_STATE_STOPPED) in gst_player_main, which gives the cleanup function state_changed_signal_data_free the opportunity to unref GstPlayer in the self->thread thread, causing the above error,we need to think about it carefully :(
Created attachment 372907 [details] [review] proposed patch Perhaps the above description of "So we call change_state (self, GST_PLAYER_STATE_STOPPED) in gst_player_main, which gives the cleanup function state_changed_signal_data_free the opportunity to unref GstPlayer in the self->thread thread" is a little inaccurate, causing unref to enter the same thread self->thread is not the only place,other places should be possible. It may not be a thorough solution,When it is really the same thread, maybe we can try to avoid calling g_thread_join (not sure whether thread resources will leak).
Unfortunately, there will be leak in the above patch. When running /quit program for a long time, I observed memory continuous increase.:(
And this is the full stack backtace: ---
+ Trace 238656
Thread 8 (Thread 7616.16885)
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
Created attachment 372908 [details] full backtrac stack When directly paste with text, some important variable information is lost, uploaded in file form.
Review of attachment 372907 [details] [review]: ::: gst-libs/gst/player/gstplayer.c @@ +493,3 @@ g_main_loop_quit (self->loop); + if (self->thread != g_thread_self ()) + g_thread_join (self->thread); If you don't join, you need to call g_thread_unref() instead. Seems to be the best we can do here However it's suboptimal that the last reference is dropped from here, ideally your application should have the last reference in the end
> If you don't join, you need to call g_thread_unref() instead. Seems to be > the best we can do here > Yep,will resubmit a patch based on your tips. > However it's suboptimal that the last reference is dropped from here, > ideally your application should have the last reference in the end It's really weird, 99.999% it works very well, but there is still a very low probability that it will be unref here, maybe there is a race happen. However, as to how the application calls our API, since we don't explicit document here,so we can't assume that. :)
Created attachment 372910 [details] [review] proposed patch
commit d5aabd0f58a7fe4c9b0b38303c7952cfc95c1283 (HEAD -> master) Author: Roland Jon <rlandjon@gmail.com> Date: Mon Jul 2 19:09:19 2018 +0800 player: Avoid trying to join the player thread from itself https://bugzilla.gnome.org/show_bug.cgi?id=796731