Created attachment 36702 [details] [review]
test

Testcase reproducing this bug.

Running only part 2 of the test shows that gst_thread_change_state() is never
called, and thus gst_thread_main_loop is never started, which causes the hang.
That's weird, because the child state change is being picked up:

DEBUG (0x9ee4848 - 307513:10:20.196742000)      GST_STATES(19266)
gstbin.c(714):gst_bin_child_state_change_func:<p> highest child state is READY,
changing bin state accordingly
LOG   (0x9ee4848 - 307513:10:20.196859000)      GST_STATES(19266)
gstbin.c(904):gst_bin_change_state_norecurse:<p> setting bin's own state

The reason appears to be that gst_bin_change_state_norecurse() calls
parent_class->change_state(bin) instead of GST_ELEMENT_GET_CLASS
(bin)->change_state (bin).

Changing that starts getting us into pitfalls, though, as setting fakesrc in a
thread to PLAYING locks up the application:

DEBUG (0x8f74848 - 307513:36:24.632583000)      GST_THREAD( 2648)
gstthread.c(428):gst_thread_change_state:<p> changing state from NULL to READY
LOG   (0x8f74848 - 307513:36:24.632676000)      GST_THREAD( 2648)
gstthread.c(436):gst_thread_change_state:<p> grabbing lock
DEBUG (0x8f74848 - 307513:36:24.632766000)      GST_THREAD( 2648)
gstthread.c(391):gst_thread_sync:<p> syncing thread, grabbing lock
LOG   (0x8f74848 - 307513:36:24.632855000)      GST_THREAD( 2648)
gstthread.c(409):gst_thread_sync:<p> caught thread

Which seems to indicate we're hanging in "g_mutex_lock (thread->iterate_lock);"
in gst_thread_change_state(), probably because of recursive state changes. The
logs indicate the same thing: setting src to READY makes the thread go to READY,
which des a recursive (!) state-cgange on the bin, which sets all kids to READY,
which sets the sink to READY, which sets the parent to READY because that didn't
complete yet (still same context!), which re-locks the mutex, hang. We need a
separate function or better code for non-recursive state changes on bins to fix
this.

My next step was to add a no-recurse flag to GstBin to make it not recurse onto
children when doing a non-recursive state-change (so this flag would be set in
gst_bin_change_state_norecurse()). I'm getting further now, my pipeline actually
starts to play again, and it even EOS'es. However, somewhere in between there
and going to NULL, it hangs.

More tomorrow...

(Note that the FIXME at the bottom of GstBin's _change_state() is very true:
setting a recursive state-change will set the bin's state already through the
kid's states, which may not be what you want. This works, though, so it's not a
bad thing.)

Created attachment 36705 [details] [review]
patch

No sleep for me. Above patch fixes the hangs for me, plus the hangs on downward
state changes. It looks large, but it isn't, it just adds some flags around
state changes and changes indenting here and there, plus that it uses an
element's class instead of the file's parent_class in a particular virtual
function call.

Created attachment 36720 [details] [review]
better patch

Obviously, make check had to fail at some point. Attached patch fixes this. The
reason is quite surprising: "threadsafe property locking" depended on recursive
behaviour of non-recursive state changes (i.e. it depended on a bug), which is
not good. This patch fixes that. Make check works, and two more tests have been
enabled, the one in my first post here and the one from #142588.

Created attachment 36728 [details] [review]
one more fix

Testsuite runs, but the media testsuite had one problem - our infamous
multi.ogg. Now I finally found out why; it destroys threads in their own
context! Ouch. Bummer we never noticed before. Anyway, it should be possible,
so this patch adds another bit of code (or, rather, removes a bit of code) to
make that possible. Now, both the core testsuite and the media testsuite runs
fine.

Applied.