Bug 797260 – shmsink: do not render if element is unlocked

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 797260 - shmsink: do not render if element is unlocked


Summary:	shmsink: do not render if element is unlocked


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-plugins-bad
Version:	1.14.x
Hardware:	Other Linux

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

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2018-10-08 20:55 UTC by Aleix Conchillo Flaqué
Modified:	2018-10-10 16:07 UTC

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

Attachments
do not block if element is unlocked (1.10 KB, patch) 2018-10-08 20:58 UTC, Aleix Conchillo Flaqué	committed	Details \| Review

Description Aleix Conchillo Flaqué 2018-10-08 20:55:36 UTC

A race condition might occur in shmsink when wait-for-connecition is TRUE and the pipeline is going to NULL.

The unlock() function will be called, but unlocking doesn't mean the render() function can't be called again which in shmsink would could cause other locks to happen (because of the conditional variable).

Since unlock() has already happened, the conditional variable will be waiting and this will block the streaming thread not letting the pipeline to go to NULL.

+ Trace 238699

Thread 3 (Thread 0x7ff799ed2700 (LWP 30791))

#0 syscall
at ../sysdeps/unix/sysv/linux/x86_64/syscall.S line 38
#1 g_cond_wait
at gthread-posix.c line 1402
#2 gst_shm_sink_render
at gstshmsink.c line 674
#3 gst_base_sink_chain_unlocked
at gstbasesink.c line 3546
#4 gst_base_sink_chain_main
at gstbasesink.c line 3672
#5 gst_base_sink_chain
at gstbasesink.c line 3701
#6 gst_pad_chain_data_unchecked
at gstpad.c line 4322
#7 gst_pad_push_data
at gstpad.c line 4578
#8 gst_pad_push
at gstpad.c line 4697
#9 gst_base_src_loop
at gstbasesrc.c line 2957
#10 gst_task_func
at gsttask.c line 332
#11 g_thread_pool_thread_proxy
at gthreadpool.c line 307
#12 g_thread_proxy
at gthread.c line 784
#13 start_thread
at pthread_create.c line 333
#14 clone
at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 109

Thread 1 (Thread 0x7ff7a0e33700 (LWP 30789))

#0 syscall
at ../sysdeps/unix/sysv/linux/x86_64/syscall.S line 38
#1 g_mutex_lock_slowpath
at gthread-posix.c line 1320
#2 g_mutex_lock
at gthread-posix.c line 1344
#3 gst_base_sink_change_state
at gstbasesink.c line 5161
#4 gst_element_change_state
at gstelement.c line 2952
#5 gst_element_set_state_func
at gstelement.c line 2906
#6 gst_bin_element_set_state
at gstbin.c line 2604
#7 gst_bin_change_state_func
at gstbin.c line 2946
#8 gst_pipeline_change_state
at gstpipeline.c line 508
#9 gst_element_change_state
at gstelement.c line 2952
#10 gst_element_set_state_func
at gstelement.c line 2906
#11 test_shm_live
at elements/shm.c line 218
#12 tcase_run_tfun_fork
at check_run.c line 465
#13 srunner_iterate_tcase_tfuns
at check_run.c line 237
#14 srunner_run_tcase
at check_run.c line 377
#15 srunner_iterate_suites
at check_run.c line 205
#16 srunner_run_tagged
at check_run.c line 740
#17 srunner_run
at check_run.c line 754
#18 srunner_run_all
at check_run.c line 692
#19 gst_check_run_suite
at gstcheck.c line 1067
#20 main
at elements/shm.c line 249

Comment 1 Aleix Conchillo Flaqué 2018-10-08 20:58:47 UTC

Created attachment 373873 [details] [review]
do not block if element is unlocked

Not sure this is the right way to do this. We just not allow more locks if the element is unlocked.

Comment 2 Josep Torra Valles 2018-10-09 16:19:43 UTC

I'd tested it and it looks correct to me.

Comment 3 Olivier Crête 2018-10-10 16:06:55 UTC

Patch looks good, merging!