Bug 747275 – jackaudiosink: hangs when jackd exits

After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.

Bug 747275 - jackaudiosink: hangs when jackd exits


Summary:	jackaudiosink: hangs when jackd exits


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-plugins-good
Version:	1.6.0
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	1.8.3
Assigned To:	GStreamer Maintainers
QA Contact:	GStreamer Maintainers

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2015-04-02 22:48 UTC by Christoph Reiter (lazka)
Modified:	2016-07-25 10:27 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
stacktrace (3.52 KB, text/plain) 2015-04-02 22:48 UTC, Christoph Reiter (lazka)		Details
Test program for reproducing the deadlock (1.46 KB, text/x-csrc) 2016-06-10 07:21 UTC, Thomas Scheuermann		Details
Patch gstjackaudiosink blocks (1.96 KB, patch) 2016-06-30 12:50 UTC, Thomas Scheuermann	reviewed	Details \| Review
Second try for the patch (2.03 KB, patch) 2016-07-08 12:26 UTC, Thomas Scheuermann	committed	Details \| Review

Description Christoph Reiter (lazka) 2015-04-02 22:48:46 UTC

Created attachment 300865 [details]
stacktrace

When playing over jackaudiosink and stopping jackd the pipeline gets an ERROR message after which I set the pipeline to NULL. This action never returns.

Sometimes it will emit EOS before ERROR in which case I also set it to NULL; also never returns.

How to reproduce:

* jackd -dalsa
* gst-play-1.0 --audiosink=jackaudiosink some_file.ogg
* killall -2 jackd

Stacktrace attached

Comment 1 Tristan Linnell 2015-11-11 16:47:13 UTC

This affects me also, using 1.6.0.

Comment 2 Tristan Linnell 2016-05-31 09:11:12 UTC

This also affects 1.6.3.

Comment 3 Thomas Scheuermann 2016-06-09 14:37:32 UTC

This affects me too.
Using 1.8.1

Comment 4 Thomas Scheuermann 2016-06-10 07:21:26 UTC

Created attachment 329532 [details]
Test program for reproducing the deadlock

The test program can be used to reproduce the deadlock. In the deadlock situation I get the following backtracesafter setting the pipeline state to GST_STATE_NULL.

(gdb) thread apply all bt

+ Trace 236317

Thread 2 (Thread 0x7fdaede68700 (LWP 30048))

#0 syscall
at ../sysdeps/unix/sysv/linux/x86_64/syscall.S line 38
#1 g_cond_wait
at /home/barco/glib2.0-2.46.0/./glib/gthread-posix.c line 1397
#2 gst_task_func
at gsttask.c line 317
#3 g_thread_pool_thread_proxy
at /home/barco/glib2.0-2.46.0/./glib/gthreadpool.c line 307
#4 g_thread_proxy
at /home/barco/glib2.0-2.46.0/./glib/gthread.c line 778
#5 start_thread
at pthread_create.c line 309
#6 clone
at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 111

Thread 1 (Thread 0x7fdaf257f700 (LWP 30037))

#0 syscall
at ../sysdeps/unix/sysv/linux/x86_64/syscall.S line 38
#1 g_cond_wait
at /home/barco/glib2.0-2.46.0/./glib/gthread-posix.c line 1397
#2 gst_jack_audio_client_set_active
at gstjackaudioclient.c line 604
#3 gst_jack_ring_buffer_release
at gstjackaudiosink.c line 544
#4 gst_audio_ring_buffer_release
at gstaudioringbuffer.c line 693
#5 gst_audio_base_sink_change_state
at gstaudiobasesink.c line 2507
#6 gst_element_change_state
at gstelement.c line 2648
#7 gst_element_set_state_func
at gstelement.c line 2602
#8 gst_bin_change_state_func
at gstbin.c line 2414
#9 gst_bin_change_state_func
at gstbin.c line 2756
#10 gst_element_change_state
at gstelement.c line 2648
#11 gst_element_change_state
at gstelement.c line 2687
#12 gst_element_set_state_func
at gstelement.c line 2602
#13 main
at deadlock.c line 46

Comment 5 Thomas Scheuermann 2016-06-30 12:50:42 UTC

Created attachment 330654 [details] [review]
Patch gstjackaudiosink blocks

This patch prevents waiting for a callback from jack when the jack server has been shut down. So stopping a pipeline with jack elements won't block anymore if the connection to the jack server was lost.

Comment 6 Sebastian Dröge (slomo) 2016-07-08 06:43:51 UTC

Review of attachment 330654 [details] [review]:

::: ext/jack/gstjackaudioclient.c
@@ +600,3 @@
   /* make sure that we are not dispatching the client */
   g_mutex_lock (&client->conn->lock);
+  if (client->active && !active && client->server_down == FALSE) {

Don't test booleans for equality. Do "!client->server_down" here.

But isn't this racy? What if it's just getting set to TRUE right after we did the check?

Comment 7 Thomas Scheuermann 2016-07-08 10:12:59 UTC

Yes, this may be racy but due to the fact that the callback will never be called if the jack server has disconnected I think it is much better than it is now.
If it gets TRUE right after the check we have the same situation as it is now.

I will change the patch with the test.

Comment 8 Sebastian Dröge (slomo) 2016-07-08 10:20:35 UTC

What I mean is that the boolean flag should be protected by the same mutex, and you should signal the condition variable whenever it changes.

The waiting for the condition variable should probably also be

>  while (client->deactivate && !client->server_down)
>    g_cond_wait(...)

Comment 9 Thomas Scheuermann 2016-07-08 12:26:59 UTC

Created attachment 331073 [details] [review]
Second try for the patch

Hi Sebastian,

I hope I got you right and made a different patch for this problem.
Please take a look.

Comment 10 Sebastian Dröge (slomo) 2016-07-08 13:52:31 UTC

commit e67f5d60abc7ecb1ad9e7ee2b29b88d56058d8e1
Author: Thomas Scheuermann <Thomas.Scheuermann@barco.com>
Date:   Thu Jun 30 14:40:40 2016 +0200

    jack: don't wait for callbacks if the jack server shut down
    
    Otherwise we'll wait forever.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=747275